feat(nestjs): Support WebSocket errors in SentryGlobalFilter#22224
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
e066783 to
6b0be86
Compare
chargome
left a comment
chargome
left a comment
There was a problem hiding this comment.
Looks good overall, just some minor nits.
nicohrubec
left a comment
nicohrubec
left a comment
There was a problem hiding this comment.
did you run the e2e test locally? the reason I closed the original PR was that I couldn't make the e2e test pass reliably locally and it's also failing in CI now
|
@psh4607 pinging you in case you want to push this further, otherwise we'll take over the pr next week |
f51a650 to
31bc079
Compare
|
Thanks for flagging this. The timeout was deterministic: Nest does not apply global |
|
@chargome Sorry for the delayed follow-up — I was away over the holiday break. This is my first contribution to sentry-javascript, so I’m excited to see it through. I’ve addressed the review comments and fixed the E2E issue. Thanks for your patience! |
5be0bfb to
503238f
Compare
nicohrubec
left a comment
nicohrubec
left a comment
There was a problem hiding this comment.
thanks for the updates, lgtm!
|
Thank you both, @nicohrubec and @chargome, for the thoughtful reviews and all the helpful feedback! I’m really happy to see my first contribution to sentry-javascript reach this point. I learned a lot throughout this PR and hope to contribute again soon. See you around! 🙌 |
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #22224 Co-authored-by: nicohrubec <29484629+nicohrubec@users.noreply.github.com>
3 participants
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Fixes #16067
SentryGlobalFilternow handles WebSocket exceptions without delegating them to Nest's HTTPBaseExceptionFilter.Unexpected gateway errors are captured with the
auto.ws.nestjs.global_filtermechanism and emit a generic error response. ExpectedWsExceptionresponses are preserved without being reported to Sentry.WsExceptionis detected by shape so@nestjs/websocketsdoes not become a required dependency.Unit and NestJS WebSocket E2E coverage was added for unexpected errors, expected
WsExceptionvalues, and HTTP exceptions raised in a WebSocket context.Root cause: WebSocket exceptions were delegated to an HTTP exception filter which expects an HTTP adapter and cannot correctly respond through a WebSocket client.