-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Sentry client setup #5297
Fix Sentry client setup #5297
Conversation
8588c4c
to
0f7ee99
Compare
Latest k6 run output1
Footnotes
|
Full-stack documentation: https://docs.openverse.org/_preview/5297 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
@@ -108,7 +108,7 @@ | |||
"npm-run-all2": "^7.0.0", | |||
"nuxt": "^3.14.1592", | |||
"rimraf": "^6.0.1", | |||
"storybook": "^8.3.6", | |||
"storybook": "8.3.6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI was failing because it was using the latest Storybook version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a weird error to have!
|
||
return await $fetch<MediaProvider[]>( | ||
`${apiUrl}v1/${mediaSlug(mediaType)}/stats/`, | ||
{ | ||
headers: { | ||
...getProxyRequestHeaders(event), | ||
...userAgentHeader, | ||
"Content-Type": "application/json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the headers to handle the fluke when sometimes the API returns HTML response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, appreciate the additional button to test this from the preferences page.
consola.info(`Fetching sources for ${mediaType} media`) | ||
consola.info(`Fetching ${mediaType} sources.`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the logs more grammatically correct 😁.
Fixes
Fixes #5296 by @obulat
Description
This PR changes the way the
release
value is set for the Sentry module on the client. It uses asentryRelease
value in thenuxtConfig
set to theprocess.env.SEMANTIC_VERSION
. Using a different name for the variable (i.e., not the correspondingNUXT_PUBLIC_SENTRY_RELEASE
one) makes sure that the value is not changed during the runtime.This PR also adds a
platform
tag to easily filter server/client events in Sentry UI.Testing Instructions
To test that the release is baked in during the build time, you can use different values for
SEMANTIC_VERSION
env variable during build/runtime. To do that:ov env SEMANTIC_VERSION=build just frontend/run build
ov env SEMANTIC_VERSION=start just frontend/run build
http://localhost:8443/preferences
, scroll down to see the "Capture Sentry Exception" button and click it. In the devtools, check the payload of the Sentry POST request. It should haverelease: "build"
in it.Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin