-
-
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
Missing sendDefaultPii
type from options
#2163
Comments
Yes, however as you already mentioned, it's not very well documented, sorry for that. sentry-javascript/packages/node/src/handlers.ts Lines 149 to 170 in 5d670a7
If you use express or any other framework making use of our handlers app.use(Sentry.Handlers.requestHandler({
user: ['keys', 'to', 'attach', 'with', 'the', 'event']
}) However, if you use SDK with raw node app, then we don't collect that data at all.
Agree, we can definitely make it better. Cheers! |
First I thought I'm going crazy, but apparently if you choose Node rather than Javascript it's not showing up as not available: https://docs.sentry.io/error-reporting/configuration/?platform=node But this repo is the place for the unified JS client including Node, right? Anyway, adding the config to the Thanks a lot for the help and explanation! |
Oh snap, that's a bug then! Will fix it, thanks!
Correct :)
Glad it helped! Cheers |
Related: getsentry/sentry-docs#1111 |
Also updated current docs for handlers getsentry/sentry-docs#1093 |
Still not showing up for nodejs; I just hit this today. |
It seems something has changed, so warning is missing again, both on JavaScript and Node.js docs. |
I've also seen this regression today for |
[] Provide a link to the affected event from your Sentry accountPackage + Version
@sentry/browser
@sentry/node
raven-js
raven-node
(raven for node)Version:
Description
The option
sendDefaultPii
does not exist in types (in fact I can not find it in this repo at all apart from being mentioned in one unrelated issue), so I'm getting a TypeScript error.Also, is it the replacement for the previous
parseUser
property? The documentation is very vague, not even listing which fields will it pick, but only sayingcertain personally identifiable information is added
.I guess some other fields might be missing too, which was masked by having a catch-all field earlier, which is now removed by #2111
The text was updated successfully, but these errors were encountered: