Skip to content

fix(nextjs): Use domains to prevent scope bleed on backend - #3574

Merged
lobsterkatie merged 1 commit into
kmclb-next-js-performancefrom
kmclb-nextjs-no-scope-bleed
May 20, 2021
Merged

fix(nextjs): Use domains to prevent scope bleed on backend#3574
lobsterkatie merged 1 commit into
kmclb-next-js-performancefrom
kmclb-nextjs-no-scope-bleed

Conversation

@lobsterkatie

Copy link
Copy Markdown
Member

Prior to this PR, every request to the backend was handled in the same global scope, causing all concurrent requests to share the same hub/scope/etc, with predictably bad consequences (in this case, primarily spans and errors getting associated with the wrong transactions, but the problems would extend to any data stored on the scope which is different for each request).

This PR surrounds everything in our wrapped version of the request handler in a domain, thereby isolating each request's global scope.

@github-actions

github-actionsBot commented May 20, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.76 KB (+0.01% 🔺)
@sentry/browser - Webpack21.98 KB (0%)
@sentry/react - Webpack22.01 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)28.16 KB (+0.01% 🔺)

@lobsterkatie
lobsterkatieforce-pushed the kmclb-nextjs-no-scope-bleed branch from d84a4bb to c3e5eccCompareMay 20, 2021 01:42

@AbhiPrasadAbhiPrasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment not related to the current changes, but something I noticed while reading the code. Feel free to resolve and come back to it later though (as it's technically out of scope for this patch).

Comment threadpackages/nextjs/src/utils/instrumentServer.ts
@lobsterkatie
lobsterkatie merged commit da07ea7 into kmclb-next-js-performanceMay 20, 2021
@lobsterkatie
lobsterkatie deleted the kmclb-nextjs-no-scope-bleed branch May 20, 2021 18:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@lobsterkatie@AbhiPrasad