feat: Auto-correct Sentry Browser SDK content type - #471
Conversation
This PR automatically corrects Sentry Browser SDK events' content type as it avoids using the standard `application/x-sentry-envelope` for CORS reasons. This also comes with the side effect of fixing URL parsing when it contains a query string or when a content-type header is not set.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| let contentType = req.headers['content-type']?.split(';')[0].toLocaleLowerCase(); | ||
| if (searchParams?.get('sentry_client')?.startsWith('sentry.javascript.browser') && req.headers.origin) { | ||
| // This is a correction we make as Sentry Browser SDK may send messages with text/plain to avoid CORS issues | ||
| contentType = 'application/x-sentry-envelope'; |
There was a problem hiding this comment.
out of curiousity: What kind of problem did this previously cause?
There was a problem hiding this comment.
I've been trying to use the standalone mode for end to end tracing in a Rails app. Since Spotlight was not on the same page, the Sentry integration did not work properly. I then decided to use the hacked DSN approach to force enable the browser SDK which sent the events with text/plain;charset=UTF-8 content type. Sidecar simply passed this down to the overlay. The Sentry integration there only listens to the envelope content type so doesn't even get triggered. I tried adding text/plain but that didn't work either way we didn't strip the charset part or do case normalization. It also felt wrong to hook onto text/plain. Hence this solution 😅
There was a problem hiding this comment.
Ahh this makes a lot of sense. Reminds me that we should export the sentry integration from the @spotlight/spotlight(?) package so that users can register it in their browser SDK setup. #403
There was a problem hiding this comment.
Was gonna look into that but I also noticed there's already a Spotlight integration in the node SDK: https://github.com/getsentry/sentry-javascript/blob/develop/packages/node/src/integrations/spotlight.ts#L6
Maybe just move that into common, enable when spotlight: true is passed to init and remove the one from Spotlight? I feel like that's the better approach. WDYT?
There was a problem hiding this comment.
Maybe just move that into common, enable when spotlight: true is passed to init and remove the one from Spotlight?
We can't do this in the browser because this means the spotlight integration would be part of the bundle. Bundlers can't tree shake the integration if we control adding it with a runtime flag like spotlight: true.
So for browser SDKs, users will always need to register the integration in Sentry.init (like browserTracingIntegration today).
Also I don't think we can easily hoist the same integration into core because the sending mechanism (node http vs fetch) will differ.
There was a problem hiding this comment.
RE exporting it from @sentry/browser vs @spotlightjs/spotlight I don't particularly care. We can also export it from the SDKs, as long as we keep things opt-in and tree-shakeable.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @spotlightjs/sidecar@1.7.0 ### Minor Changes - Auto-correct Sentry Browser SDK content type & fix URL matching with query string ([#471](#471)) ## @spotlightjs/spotlight@2.3.0 ### Minor Changes - Add `#debug` flag to standalone Spotlight web UI ([#469](#469)) ### Patch Changes - Updated dependencies \[[`0f10d81d60bde5753921094c7f7d7c5aa71acbaf`](0f10d81), [`7837149f0ce2de84a84eb1e0df7c6e4f317741d0`](7837149)]: - @spotlightjs/overlay@2.2.1 - @spotlightjs/sidecar@1.7.0 ## @spotlightjs/astro@2.1.5 ### Patch Changes - Updated dependencies \[[`b7774ef2fdb924c25c6e8365a3aaf20acf926f4d`](b7774ef)]: - @spotlightjs/spotlight@2.3.0 ## @spotlightjs/electron@1.1.3 ### Patch Changes - Updated dependencies \[[`0f10d81d60bde5753921094c7f7d7c5aa71acbaf`](0f10d81), [`7837149f0ce2de84a84eb1e0df7c6e4f317741d0`](7837149)]: - @spotlightjs/overlay@2.2.1 - @spotlightjs/sidecar@1.7.0 ## @spotlightjs/overlay@2.2.1 ### Patch Changes - Fix event processing pipeline for integrations ([#470](#470)) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This PR automatically corrects Sentry Browser SDK events' content type as it avoids using the standard `application/x-sentry-envelope` for CORS reasons. This also comes with the side effect of fixing URL parsing when it contains a query string or when a content-type header is not set.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @spotlightjs/sidecar@1.7.0 ### Minor Changes - Auto-correct Sentry Browser SDK content type & fix URL matching with query string ([#471](#471)) ## @spotlightjs/spotlight@2.3.0 ### Minor Changes - Add `#debug` flag to standalone Spotlight web UI ([#469](#469)) ### Patch Changes - Updated dependencies \[[`0f10d81d60bde5753921094c7f7d7c5aa71acbaf`](0f10d81), [`7837149f0ce2de84a84eb1e0df7c6e4f317741d0`](7837149)]: - @spotlightjs/overlay@2.2.1 - @spotlightjs/sidecar@1.7.0 ## @spotlightjs/astro@2.1.5 ### Patch Changes - Updated dependencies \[[`b7774ef2fdb924c25c6e8365a3aaf20acf926f4d`](b7774ef)]: - @spotlightjs/spotlight@2.3.0 ## @spotlightjs/electron@1.1.3 ### Patch Changes - Updated dependencies \[[`0f10d81d60bde5753921094c7f7d7c5aa71acbaf`](0f10d81), [`7837149f0ce2de84a84eb1e0df7c6e4f317741d0`](7837149)]: - @spotlightjs/overlay@2.2.1 - @spotlightjs/sidecar@1.7.0 ## @spotlightjs/overlay@2.2.1 ### Patch Changes - Fix event processing pipeline for integrations ([#470](#470)) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This PR automatically corrects Sentry Browser SDK events' content type as it avoids using the standard
application/x-sentry-envelopefor CORS reasons. This also comes with the side effect of fixing URL parsing when it contains a query string or when a content-type header is not set.