Skip to content
Open
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 { MessageBody, SubscribeMessage, WebSocketGateway, 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 WebSocket 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 unexpected errors in WebSocket gateway handlers', 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',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
value: 'This is an exception in a WebSocket handler',
value: 'This is an exception in a WebSocket handler',
mechanism: {
handled: false,
type: 'auto.ws.nestjs.global_filter',
},

});

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 an expected exception which won't 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,
// 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 automatically capture expected WebSocket exceptions', 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 WebSocket 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';
}
29 changes: 28 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,33 @@ class SentryGlobalFilter extends BaseExceptionFilter {
return;
}

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Let's put the emit function optional here, in case a user uses a ws adapter that is not compatible to this interface.


if (isWsException(exception)) {
const result = (exception as { getError: () => unknown }).getError();
const response = typeof result === 'object' && result !== null ? result : { status: 'error', message: result };
client.emit('exception', response);
return;
}

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
66 changes: 66 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,67 @@ describe('SentryGlobalFilter', () => {
expect(mockCaptureException).not.toHaveBeenCalled();
});
});

describe('WebSocket context', () => {
let mockEmit: ReturnType<typeof vi.fn>;

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

it('captures unexpected errors and emits a generic error response', () => {
const error = new Error('Test WebSocket 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('does not capture expected WebSocket exceptions and emits their response', () => {
isExpectedErrorMock.mockReturnValueOnce(true);
isWsExceptionMock.mockReturnValueOnce(true);
const exception = {
getError: () => 'Expected WebSocket exception',
initMessage: vi.fn(),
};

filter.catch(exception, mockArgumentsHost);

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

it('does not capture HTTP exceptions and emits a generic error response', () => {
isExpectedErrorMock.mockReturnValueOnce(true);
const exception = new HttpException('Bad request', HttpStatus.BAD_REQUEST);

filter.catch(exception, mockArgumentsHost);

expect(mockCaptureException).not.toHaveBeenCalled();
expect(mockLoggerError).toHaveBeenCalledWith(exception.message, exception.stack);
expect(mockEmit).toHaveBeenCalledWith('exception', {
status: 'error',
message: 'Internal server error',
});
});
});
});
Loading