Skip to content
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

Express request.user is not used since 8.40.0 #14847

Closed
3 tasks done
richardsimko opened this issue Dec 28, 2024 · 6 comments · Fixed by #14850 or #14851
Closed
3 tasks done

Express request.user is not used since 8.40.0 #14847

richardsimko opened this issue Dec 28, 2024 · 6 comments · Fixed by #14850 or #14851
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@richardsimko
Copy link
Contributor

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.47.0

Framework Version

Express 5.0.1

Link to Sentry event

https://fl101.sentry.io/issues/6062464177/events/77ae0d4d27ca49da82cb9f19d8bca736/?project=4505209444958208

Reproduction Example/SDK Setup

We're running a web app with Express and Sentry tracked User ID and email automatically (Based on req.user I would assume?) until 8.40.0. Since then it has been broken and the docs don't really provide any guidance for how to properly set the user ID in the context of a request.

Steps to Reproduce

  1. Set up an Express app with Sentry <8.40.0
  2. Validate the User is set
  3. Upgrade Sentry
  4. User is no longer set

Expected Result

User is set or documentation is provided for how to do it manually as well as a note in the changelog that this is no longer done automatically.

Actual Result

User is not set

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 28, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Dec 28, 2024
@mydea
Copy link
Member

mydea commented Dec 30, 2024

Thank you for writing in!

First of all, you are right that our docs are lacking here. I opened a PR to improve on this, explaining how you can/should set the user yourself: getsentry/sentry-docs#12224

Generally speaking, I would recommend you to do this anyhow, as in v9 we will def. remove the default user setting from the request - see #14806.

As of now (v8), this is kind of an incidental thing. I would classify it as a bug that the user is not picked up anymore, as you described 😬 although this is not fully specced out anywhere, we used to take it, so this should continue to work on v8. I'll look into this to find out why this is happening, it is probably related to #14401 or #14315.

@mydea
Copy link
Member

mydea commented Dec 30, 2024

Could you share your express app setup, e.g. which middleware(s) are you using?

@mydea mydea changed the title User tracking broken since 8.40 Express request.user is not used since 8.40.0 Dec 30, 2024
@richardsimko
Copy link
Contributor Author

Thanks, I'll have a look at the docs PR!

We have a very long list of middleware 😅 But relevant for this we use express-session which is the one that will set req.session.user to the user ID, I would presume that's what Sentry was picking up?

If Sentry.setUser() is always bound to the current request then it's very easy for us to set manually, I just wasn't sure if that was actually properly isolated these days, we've ha problems with race conditions relating to that before (Although that was several years ago at this point). And there was no indication of how it works in the docs, although I see that your PR fixes that, thanks!

@mydea
Copy link
Member

mydea commented Dec 30, 2024

Thanks for the information!

I think I have a fix pending here: #14850, I hope this will resolve the issue!
But I would still encourage you to update to setting this manually, as this will change in v9, so you'll be ahead of the curve then ;)

@richardsimko
Copy link
Contributor Author

Thanks for the fast feedback and resolution!

Copy link
Contributor

github-actions bot commented Jan 7, 2025

A PR closing this issue has just been released 🚀

This issue was referenced by PR #14851, which was included in the 8.48.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK
Projects
Archived in project
2 participants