Skip to content

Commit f9374da

Browse files
committed
capture ws exceptions
1 parent ec13b40 commit f9374da

5 files changed

Lines changed: 157 additions & 9 deletions

File tree

dev-packages/e2e-tests/test-applications/nestjs-websockets/src/app.gateway.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { SubscribeMessage, WebSocketGateway, MessageBody } from '@nestjs/websockets';
1+
import { SubscribeMessage, WebSocketGateway, MessageBody, WsException } from '@nestjs/websockets';
22
import * as Sentry from '@sentry/nestjs';
33

44
@WebSocketGateway()
@@ -8,6 +8,11 @@ export class AppGateway {
88
throw new Error('This is an exception in a WebSocket handler');
99
}
1010

11+
@SubscribeMessage('test-ws-exception')
12+
handleWsException() {
13+
throw new WsException('Expected WS exception');
14+
}
15+
1116
@SubscribeMessage('test-manual-capture')
1217
handleManualCapture() {
1318
try {

dev-packages/e2e-tests/test-applications/nestjs-websockets/tests/errors.test.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,36 @@ test('Captures manually reported error in WebSocket gateway handler', async ({ b
2222
socket.disconnect();
2323
});
2424

25+
test('Automatically captures exceptions in WebSocket gateway handler', async ({ baseURL }) => {
26+
const errorPromise = waitForError('nestjs-websockets', event => {
27+
return event.exception?.values?.[0]?.value === 'This is an exception in a WebSocket handler';
28+
});
29+
30+
const socket = io(baseURL!);
31+
await new Promise<void>(resolve => socket.on('connect', resolve));
32+
33+
socket.emit('test-exception', {});
34+
35+
const error = await errorPromise;
36+
37+
expect(error.exception?.values?.[0]).toMatchObject({
38+
type: 'Error',
39+
value: 'This is an exception in a WebSocket handler',
40+
});
41+
42+
socket.disconnect();
43+
});
44+
2545
// There is no good mechanism to verify that an event was NOT sent to Sentry.
26-
// The idea here is that we first send a message that triggers an exception which won't be auto-captured,
46+
// The idea here is that we first send a message that triggers a WsException which should not be auto-captured,
2747
// and then send a message that triggers a manually captured error which will be sent to Sentry.
28-
// If the manually captured error arrives, we can deduce that the first exception was not sent,
48+
// If the manually captured error arrives, we can deduce that the WsException was not sent,
2949
// because Socket.IO guarantees message ordering: https://socket.io/docs/v4/delivery-guarantees
30-
test('Does not automatically capture exceptions in WebSocket gateway handler', async ({ baseURL }) => {
50+
test('Does not capture WsException in WebSocket gateway handler', async ({ baseURL }) => {
3151
let errorEventOccurred = false;
3252

3353
waitForError('nestjs-websockets', event => {
34-
if (!event.type && event.exception?.values?.[0]?.value === 'This is an exception in a WebSocket handler') {
54+
if (!event.type && event.exception?.values?.[0]?.value === 'Expected WS exception') {
3555
errorEventOccurred = true;
3656
}
3757

@@ -45,7 +65,7 @@ test('Does not automatically capture exceptions in WebSocket gateway handler', a
4565
const socket = io(baseURL!);
4666
await new Promise<void>(resolve => socket.on('connect', resolve));
4767

48-
socket.emit('test-exception', {});
68+
socket.emit('test-ws-exception', {});
4969
socket.emit('test-manual-capture', {});
5070
await manualCapturePromise;
5171

packages/nestjs/src/helpers.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,25 @@ export function isExpectedError(exception: unknown): boolean {
2525
return true;
2626
}
2727

28-
// RpcException
29-
if (typeof ex.getError === 'function' && typeof ex.initMessage === 'function') {
28+
// RpcException / WsException (same duck-type shape)
29+
if (isWsException(exception)) {
3030
return true;
3131
}
3232

3333
return false;
3434
}
35+
36+
/**
37+
* Determines if the exception is a WsException (or RpcException, which has the same shape).
38+
* Both have `getError()` and `initMessage()` methods.
39+
*
40+
* We use duck-typing to avoid importing from `@nestjs/websockets` or `@nestjs/microservices`.
41+
*/
42+
export function isWsException(exception: unknown): boolean {
43+
if (typeof exception !== 'object' || exception === null) {
44+
return false;
45+
}
46+
47+
const ex = exception as Record<string, unknown>;
48+
return typeof ex.getError === 'function' && typeof ex.initMessage === 'function';
49+
}

packages/nestjs/src/setup.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Catch, Global, HttpException, Injectable, Logger, Module } from '@nestj
1010
import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core';
1111
import { captureException, debug, getDefaultIsolationScope, getIsolationScope } from '@sentry/core';
1212
import type { Observable } from 'rxjs';
13-
import { isExpectedError } from './helpers';
13+
import { isExpectedError, isWsException } from './helpers';
1414

1515
// Partial extract of FastifyRequest interface
1616
// https://github.com/fastify/fastify/blob/87f9f20687c938828f1138f91682d568d2a31e53/types/request.d.ts#L41
@@ -152,6 +152,39 @@ class SentryGlobalFilter extends BaseExceptionFilter {
152152
return;
153153
}
154154

155+
// Handle WebSocket context
156+
if (contextType === 'ws') {
157+
if (exception instanceof HttpException) {
158+
throw exception;
159+
}
160+
161+
if (!isExpectedError(exception)) {
162+
captureException(exception, {
163+
mechanism: {
164+
handled: false,
165+
type: 'auto.ws.nestjs.global_filter',
166+
},
167+
});
168+
}
169+
170+
const client: { emit: (event: string, data: unknown) => void } = host.switchToWs().getClient();
171+
172+
// WsException: extract error and emit to client
173+
if (isWsException(exception)) {
174+
const res = (exception as { getError(): unknown }).getError();
175+
const message = typeof res === 'object' && res !== null ? res : { status: 'error', message: res };
176+
client.emit('exception', message);
177+
return;
178+
}
179+
180+
// Unknown error: log and emit generic error
181+
if (exception instanceof Error) {
182+
this._logger.error(exception.message, exception.stack);
183+
}
184+
client.emit('exception', { status: 'error', message: 'Internal server error' });
185+
return;
186+
}
187+
155188
// HTTP exceptions
156189
if (!isExpectedError(exception)) {
157190
captureException(exception, {

packages/nestjs/test/sentry-global-filter.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { SentryGlobalFilter } from '../src/setup';
88

99
vi.mock('../src/helpers', () => ({
1010
isExpectedError: vi.fn(),
11+
isWsException: vi.fn(),
1112
}));
1213

1314
vi.mock('@sentry/core', () => ({
@@ -27,6 +28,7 @@ describe('SentryGlobalFilter', () => {
2728
let mockLoggerError: any;
2829
let mockLoggerWarn: any;
2930
let isExpectedErrorMock: any;
31+
let isWsExceptionMock: any;
3032

3133
beforeEach(() => {
3234
vi.clearAllMocks();
@@ -57,6 +59,7 @@ describe('SentryGlobalFilter', () => {
5759
mockCaptureException = vi.spyOn(SentryCore, 'captureException').mockReturnValue('mock-event-id');
5860

5961
isExpectedErrorMock = vi.mocked(Helpers.isExpectedError).mockImplementation(() => false);
62+
isWsExceptionMock = vi.mocked(Helpers.isWsException).mockImplementation(() => false);
6063
});
6164

6265
describe('HTTP context', () => {
@@ -237,4 +240,76 @@ describe('SentryGlobalFilter', () => {
237240
expect(mockCaptureException).not.toHaveBeenCalled();
238241
});
239242
});
243+
244+
describe('WS context', () => {
245+
let mockEmit: any;
246+
let mockClient: any;
247+
248+
beforeEach(() => {
249+
mockEmit = vi.fn();
250+
mockClient = { emit: mockEmit };
251+
vi.mocked(mockArgumentsHost.getType).mockReturnValue('ws');
252+
vi.mocked(mockArgumentsHost.switchToWs).mockReturnValue({
253+
getClient: () => mockClient,
254+
getData: vi.fn(),
255+
getPattern: vi.fn(),
256+
} as any);
257+
});
258+
259+
it('should capture unexpected errors and emit generic error to client', () => {
260+
const error = new Error('Test WS error');
261+
262+
filter.catch(error, mockArgumentsHost);
263+
264+
expect(mockCaptureException).toHaveBeenCalledWith(error, {
265+
mechanism: {
266+
handled: false,
267+
type: 'auto.ws.nestjs.global_filter',
268+
},
269+
});
270+
expect(mockLoggerError).toHaveBeenCalledWith(error.message, error.stack);
271+
expect(mockEmit).toHaveBeenCalledWith('exception', { status: 'error', message: 'Internal server error' });
272+
});
273+
274+
it('should not capture expected WsException errors and emit extracted error to client', () => {
275+
isExpectedErrorMock.mockReturnValueOnce(true);
276+
isWsExceptionMock.mockReturnValueOnce(true);
277+
278+
const wsException = {
279+
getError: () => ({ status: 'error', message: 'Expected WS exception' }),
280+
initMessage: () => {},
281+
};
282+
283+
filter.catch(wsException, mockArgumentsHost);
284+
285+
expect(mockCaptureException).not.toHaveBeenCalled();
286+
expect(mockEmit).toHaveBeenCalledWith('exception', { status: 'error', message: 'Expected WS exception' });
287+
});
288+
289+
it('should wrap string error from WsException in object', () => {
290+
isExpectedErrorMock.mockReturnValueOnce(true);
291+
isWsExceptionMock.mockReturnValueOnce(true);
292+
293+
const wsException = {
294+
getError: () => 'string error',
295+
initMessage: () => {},
296+
};
297+
298+
filter.catch(wsException, mockArgumentsHost);
299+
300+
expect(mockCaptureException).not.toHaveBeenCalled();
301+
expect(mockEmit).toHaveBeenCalledWith('exception', { status: 'error', message: 'string error' });
302+
});
303+
304+
it('should throw HttpExceptions in WS context without capturing', () => {
305+
const httpException = new HttpException('Test HTTP exception', HttpStatus.BAD_REQUEST);
306+
307+
expect(() => {
308+
filter.catch(httpException, mockArgumentsHost);
309+
}).toThrow(httpException);
310+
311+
expect(mockCaptureException).not.toHaveBeenCalled();
312+
expect(mockEmit).not.toHaveBeenCalled();
313+
});
314+
});
240315
});

0 commit comments

Comments
 (0)