-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(nextjs): Fix devserver CORS blockage when assetPrefix
is defined
#9766
fix(nextjs): Fix devserver CORS blockage when assetPrefix
is defined
#9766
Conversation
@@ -113,7 +113,16 @@ function addClientIntegrations(options: BrowserOptions): void { | |||
if (hasTracingEnabled(options)) { | |||
const defaultBrowserTracingIntegration = new BrowserTracing({ | |||
// eslint-disable-next-line deprecation/deprecation | |||
tracingOrigins: [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/], | |||
tracingOrigins: |
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.
can/should we use tracePropagationTargets
here?
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.
I believe that would be breaking if people configured tracingOrigins
themselves because the tracePropagationTargets
would gain precedence?
// In case both are specified, tracePropagationTargets takes precedence |
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.
hmm yeah could be. let's leave it as is then!
What happens if users manualyl set TPTs? our fix for dev mode would no longer be applied, right? |
Yeah correct. This is a tricky one. The only way we could make this sorta bulletproof is to add a new option to BrowserTracing to filter out certain urls but I would like to avoid this until we land #9764. Do you have an idea on what we could do? Ofc we could map and transform tPTs or tracingOrigins but that seems messy. |
Fixes #8727
When
assetPrefix
is defined the webpack dev server will live on that prefixed domain.The SDK may attach tracing headers to the HMR server when the asset prefix is something like
http://cdn.localhost:3000
because we match on "localhost". Because this is then a different origin we run into CORS issues with the dev server.This PR resolves that by special casing the
tracePropagationTargets
when Next.js is in dev mode. This is a bit of a hack for now.Once we get to #9764 we can remove the hack again.