feat(nestjs): Support WebSocket errors in SentryGlobalFilter#22224
feat(nestjs): Support WebSocket errors in SentryGlobalFilter#22224psh4607 wants to merge 4 commits into
Conversation
e066783 to
6b0be86
Compare
chargome
left a comment
There was a problem hiding this comment.
Looks good overall, just some minor nits.
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! |
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.