-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(node): Migrate to domains used through AsyncContextStrategy
#7779
feat(node): Migrate to domains used through AsyncContextStrategy
#7779
Conversation
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.
node sdk index
is now side effect free :)
Ah, just noticed I still have some domain usages to remove from serverless! |
This makes me so happy :) Speaking of, @timfish would you mind adding a |
We also need to adjust the test in |
…ish/sentry-javascript into feat/migrate-async-strategy
Should we be re-exporting |
Shouldn't we be exporting this API everywhere? Doesn't have to be right now but at some point I think we should, right? |
Yup we'll need to re-export this API, since this is how we are going to tell people to do isolation when they manually instrument. |
I might change the name of the |
…ish/sentry-javascript into feat/migrate-async-strategy
And this is why you don't do early morning code review 😭 - it is there, just skipped the file completely! |
This PR:
domain.create().bind/run
withrunWithAsyncContext
domain.active
hack from the Next.jsinit
since domains are now not created byrunWithAsyncContext
until afterinitNode
runWithAsyncContext
from@sentry/node