Skip to content

fix(node): attach local req scope to req.sentryScope#4627

Closed
derN3rd wants to merge 1 commit intogetsentry:masterfrom
derN3rd:master
Closed

fix(node): attach local req scope to req.sentryScope#4627
derN3rd wants to merge 1 commit intogetsentry:masterfrom
derN3rd:master

Conversation

@derN3rd
Copy link

@derN3rd derN3rd commented Feb 23, 2022

This PR:

  • Attaches the scope of the domain context inside of a request to req.sentryScope
  • Adds a test to check if req.sentryScope is defined

Fixes #4607

@derN3rd
Copy link
Author

derN3rd commented Feb 23, 2022

I guess this should be also added to the docs, if this will make it into master.
https://docs.sentry.io/platforms/node/guides/express/

Will add a PR to sentry-docs afterwards

@AbhiPrasad
Copy link
Contributor

AbhiPrasad commented Feb 23, 2022

Hey thanks for opening a PR! We're gonna chat about this as a team (pinged everyone as appropriate), and then we can come back and figure out how to proceed

@AbhiPrasad
Copy link
Contributor

We are currently discussing if this would be beneficial over const scope = Sentry.getCurrentHub().getScope() - which should effectively do the same thing.

Thoughts?

@derN3rd
Copy link
Author

derN3rd commented Feb 24, 2022

Didn't notice that Sentry.getCurrentHub() is domain-aware in node envs, will try that.

If I remember correctly, I saw some issues with that in async contexts, like express-apis are, also here in the repo.
But I will try the suggested solution first in our production environment and see if there are any issues with not correctly matched data (also with my solution)

Thanks!

Sign up for free to 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.

[Feature Request] Expose Request Scope in Express Integration

2 participants