Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SubscribeMessage, WebSocketGateway, MessageBody } from '@nestjs/websockets';
import { SubscribeMessage, WebSocketGateway, MessageBody, WsException } from '@nestjs/websockets';
import * as Sentry from '@sentry/nestjs';

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

@SubscribeMessage('test-ws-exception')
handleWsException() {
throw new WsException('Expected WS exception');
}

@SubscribeMessage('test-manual-capture')
handleManualCapture() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,36 @@ test('Captures manually reported error in WebSocket gateway handler', async ({ b
socket.disconnect();
});

test('Automatically captures exceptions in WebSocket gateway handler', async ({ baseURL }) => {
const errorPromise = waitForError('nestjs-websockets', event => {
return event.exception?.values?.[0]?.value === 'This is an exception in a WebSocket handler';
});

const socket = io(baseURL!);
await new Promise<void>(resolve => socket.on('connect', resolve));

socket.emit('test-exception', {});

const error = await errorPromise;

expect(error.exception?.values?.[0]).toMatchObject({
type: 'Error',
value: 'This is an exception in a WebSocket handler',
});

socket.disconnect();
});

// There is no good mechanism to verify that an event was NOT sent to Sentry.
// The idea here is that we first send a message that triggers an exception which won't be auto-captured,
// The idea here is that we first send a message that triggers a WsException which should not be auto-captured,
// and then send a message that triggers a manually captured error which will be sent to Sentry.
// If the manually captured error arrives, we can deduce that the first exception was not sent,
// If the manually captured error arrives, we can deduce that the WsException was not sent,
// because Socket.IO guarantees message ordering: https://socket.io/docs/v4/delivery-guarantees
test('Does not automatically capture exceptions in WebSocket gateway handler', async ({ baseURL }) => {
test('Does not capture WsException in WebSocket gateway handler', async ({ baseURL }) => {
let errorEventOccurred = false;

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

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

socket.emit('test-exception', {});
socket.emit('test-ws-exception', {});
socket.emit('test-manual-capture', {});
await manualCapturePromise;

Expand Down
19 changes: 17 additions & 2 deletions packages/nestjs/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,25 @@ export function isExpectedError(exception: unknown): boolean {
return true;
}

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

return false;
}

/**
* Determines if the exception is a WsException (or RpcException, which has the same shape).
* Both have `getError()` and `initMessage()` methods.
*
* We use duck-typing to avoid importing from `@nestjs/websockets` or `@nestjs/microservices`.
*/
export function isWsException(exception: unknown): boolean {
if (typeof exception !== 'object' || exception === null) {
return false;
}

const ex = exception as Record<string, unknown>;
return typeof ex.getError === 'function' && typeof ex.initMessage === 'function';
}
35 changes: 34 additions & 1 deletion packages/nestjs/src/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Catch, Global, HttpException, Injectable, Logger, Module } from '@nestj
import { APP_INTERCEPTOR, BaseExceptionFilter } from '@nestjs/core';
import { captureException, debug, getDefaultIsolationScope, getIsolationScope } from '@sentry/core';
import type { Observable } from 'rxjs';
import { isExpectedError } from './helpers';
import { isExpectedError, isWsException } from './helpers';

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

// Handle WebSocket context
if (contextType === 'ws') {
if (exception instanceof HttpException) {
throw exception;
}

if (!isExpectedError(exception)) {
captureException(exception, {
mechanism: {
handled: false,
type: 'auto.ws.nestjs.global_filter',
},
});
}

const client: { emit: (event: string, data: unknown) => void } = host.switchToWs().getClient();

// WsException: extract error and emit to client
if (isWsException(exception)) {
const res = (exception as { getError(): unknown }).getError();
const message = typeof res === 'object' && res !== null ? res : { status: 'error', message: res };
client.emit('exception', message);
return;
}

// Unknown error: log and emit generic error
if (exception instanceof Error) {
this._logger.error(exception.message, exception.stack);
}
client.emit('exception', { status: 'error', message: 'Internal server error' });
return;
}

// HTTP exceptions
if (!isExpectedError(exception)) {
captureException(exception, {
Expand Down
75 changes: 75 additions & 0 deletions packages/nestjs/test/sentry-global-filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { SentryGlobalFilter } from '../src/setup';

vi.mock('../src/helpers', () => ({
isExpectedError: vi.fn(),
isWsException: vi.fn(),
}));

vi.mock('@sentry/core', () => ({
Expand All @@ -27,6 +28,7 @@ describe('SentryGlobalFilter', () => {
let mockLoggerError: any;
let mockLoggerWarn: any;
let isExpectedErrorMock: any;
let isWsExceptionMock: any;

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

isExpectedErrorMock = vi.mocked(Helpers.isExpectedError).mockImplementation(() => false);
isWsExceptionMock = vi.mocked(Helpers.isWsException).mockImplementation(() => false);
});

describe('HTTP context', () => {
Expand Down Expand Up @@ -237,4 +240,76 @@ describe('SentryGlobalFilter', () => {
expect(mockCaptureException).not.toHaveBeenCalled();
});
});

describe('WS context', () => {
let mockEmit: any;
let mockClient: any;

beforeEach(() => {
mockEmit = vi.fn();
mockClient = { emit: mockEmit };
vi.mocked(mockArgumentsHost.getType).mockReturnValue('ws');
vi.mocked(mockArgumentsHost.switchToWs).mockReturnValue({
getClient: () => mockClient,
getData: vi.fn(),
getPattern: vi.fn(),
} as any);
});

it('should capture unexpected errors and emit generic error to client', () => {
const error = new Error('Test WS error');

filter.catch(error, mockArgumentsHost);

expect(mockCaptureException).toHaveBeenCalledWith(error, {
mechanism: {
handled: false,
type: 'auto.ws.nestjs.global_filter',
},
});
expect(mockLoggerError).toHaveBeenCalledWith(error.message, error.stack);
expect(mockEmit).toHaveBeenCalledWith('exception', { status: 'error', message: 'Internal server error' });
});

it('should not capture expected WsException errors and emit extracted error to client', () => {
isExpectedErrorMock.mockReturnValueOnce(true);
isWsExceptionMock.mockReturnValueOnce(true);

const wsException = {
getError: () => ({ status: 'error', message: 'Expected WS exception' }),
initMessage: () => {},
};

filter.catch(wsException, mockArgumentsHost);

expect(mockCaptureException).not.toHaveBeenCalled();
expect(mockEmit).toHaveBeenCalledWith('exception', { status: 'error', message: 'Expected WS exception' });
});

it('should wrap string error from WsException in object', () => {
isExpectedErrorMock.mockReturnValueOnce(true);
isWsExceptionMock.mockReturnValueOnce(true);

const wsException = {
getError: () => 'string error',
initMessage: () => {},
};

filter.catch(wsException, mockArgumentsHost);

expect(mockCaptureException).not.toHaveBeenCalled();
expect(mockEmit).toHaveBeenCalledWith('exception', { status: 'error', message: 'string error' });
});

it('should throw HttpExceptions in WS context without capturing', () => {
const httpException = new HttpException('Test HTTP exception', HttpStatus.BAD_REQUEST);

expect(() => {
filter.catch(httpException, mockArgumentsHost);
}).toThrow(httpException);

expect(mockCaptureException).not.toHaveBeenCalled();
expect(mockEmit).not.toHaveBeenCalled();
});
});
});
Loading