Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions packages/nestjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"devDependencies": {
"@nestjs/common": "^10.0.0",
"@nestjs/core": "^10.0.0",
"@nestjs/microservices": "^10.0.0",
"reflect-metadata": "^0.2.2"
},
"peerDependencies": {
Expand Down
37 changes: 36 additions & 1 deletion packages/nestjs/src/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,12 @@ class SentryGlobalFilter extends BaseExceptionFilter {
* Catches exceptions and reports them to Sentry unless they are expected errors.
*/
public catch(exception: unknown, host: ArgumentsHost): void {
const contextType = host.getType<string>();

// The BaseExceptionFilter does not work well in GraphQL applications.
// By default, Nest GraphQL applications use the ExternalExceptionFilter, which just rethrows the error:
// https://github.com/nestjs/nest/blob/master/packages/core/exceptions/external-exception-filter.ts
if (host.getType<'graphql'>() === 'graphql') {
if (contextType === 'graphql') {
// neither report nor log HttpExceptions
if (exception instanceof HttpException) {
throw exception;
Expand All @@ -103,6 +105,39 @@ class SentryGlobalFilter extends BaseExceptionFilter {
throw exception;
}

// Handle microservice context (rpc)
// We cannot add proper handing here since RpcException depend on the @nestjs/microservices package
// For these cases we log a warning that the user should be providing a dedicated exception filter
if (contextType === 'rpc') {
// Unlikely case
if (exception instanceof HttpException) {
throw exception;
}

// Handle any other kind of error
if (!(exception instanceof Error)) {
if (!isExpectedError(exception)) {
captureException(exception);
}
throw exception;
}

// In this case we're likely running into an RpcException, which the user should handle with a dedicated filter
// https://github.com/nestjs/nest/blob/master/sample/03-microservices/src/common/filters/rpc-exception.filter.ts
if (!isExpectedError(exception)) {
captureException(exception);
}

this._logger.warn(
'IMPORTANT: RpcException should be handled with a dedicated Rpc exception filter, not the generic SentryGlobalFilter',
);

// Log the error and return, otherwise we may crash the user's app by handling rpc errors in a http context
this._logger.error(exception.message, exception.stack);
return;
}

// HTTP exceptions
if (!isExpectedError(exception)) {
captureException(exception);
}
Expand Down
235 changes: 235 additions & 0 deletions packages/nestjs/test/sentry-global-filter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
/* eslint-disable @typescript-eslint/unbound-method */
import { describe, it, expect, beforeEach, vi } from 'vitest';
import type { ArgumentsHost } from '@nestjs/common';
import { HttpException, HttpStatus, Logger } from '@nestjs/common';
import { SentryGlobalFilter } from '../src/setup';
import * as SentryCore from '@sentry/core';
import * as Helpers from '../src/helpers';

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

vi.mock('@sentry/core', () => ({
captureException: vi.fn().mockReturnValue('mock-event-id'),
getIsolationScope: vi.fn(),
getDefaultIsolationScope: vi.fn(),
logger: {
warn: vi.fn(),
},
}));

describe('SentryGlobalFilter', () => {
let filter: SentryGlobalFilter;
let mockArgumentsHost: ArgumentsHost;
let mockHttpServer: any;
let mockCaptureException: any;
let mockLoggerError: any;
let mockLoggerWarn: any;
let isExpectedErrorMock: any;

beforeEach(() => {
vi.clearAllMocks();

mockHttpServer = {
getRequestMethod: vi.fn(),
getRequestUrl: vi.fn(),
};

filter = new SentryGlobalFilter(mockHttpServer);

mockArgumentsHost = {
getType: vi.fn().mockReturnValue('http'),
getArgs: vi.fn().mockReturnValue([]),
getArgByIndex: vi.fn().mockReturnValue({}),
switchToHttp: vi.fn().mockReturnValue({
getRequest: vi.fn().mockReturnValue({}),
getResponse: vi.fn().mockReturnValue({}),
getNext: vi.fn(),
}),
switchToRpc: vi.fn(),
switchToWs: vi.fn(),
} as unknown as ArgumentsHost;

mockLoggerError = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => {});
mockLoggerWarn = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => {});

mockCaptureException = vi.spyOn(SentryCore, 'captureException').mockReturnValue('mock-event-id');

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

describe('HTTP context', () => {
beforeEach(() => {
vi.mocked(mockArgumentsHost.getType).mockReturnValue('http');
});

it('should capture non-HttpException errors and call super.catch for HTTP context', () => {
const originalCatch = filter.catch;
const superCatchSpy = vi.fn();
filter.catch = function (exception, host) {
if (!Helpers.isExpectedError(exception)) {
SentryCore.captureException(exception);
}
superCatchSpy(exception, host);
return {} as any;
};

const error = new Error('Test error');

filter.catch(error, mockArgumentsHost);

expect(mockCaptureException).toHaveBeenCalledWith(error);
expect(superCatchSpy).toHaveBeenCalled();

filter.catch = originalCatch;
});

it('should not capture expected errors', () => {
const originalCatch = filter.catch;
const superCatchSpy = vi.fn();

isExpectedErrorMock.mockReturnValueOnce(true);

filter.catch = function (exception, host) {
if (!Helpers.isExpectedError(exception)) {
SentryCore.captureException(exception);
}
superCatchSpy(exception, host);
return {} as any;
};

const expectedError = new Error('Test error');

filter.catch(expectedError, mockArgumentsHost);

expect(mockCaptureException).not.toHaveBeenCalled();
expect(superCatchSpy).toHaveBeenCalled();

filter.catch = originalCatch;
});
});

describe('GraphQL context', () => {
beforeEach(() => {
vi.mocked(mockArgumentsHost.getType).mockReturnValue('graphql');
});

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

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

expect(mockCaptureException).not.toHaveBeenCalled();
expect(mockLoggerError).not.toHaveBeenCalled();
});

it('should log and capture non-HttpException errors in GraphQL context', () => {
const error = new Error('Test error');

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

expect(mockCaptureException).toHaveBeenCalledWith(error);
expect(mockLoggerError).toHaveBeenCalledWith(error.message, error.stack);
});
});

describe('RPC context', () => {
beforeEach(() => {
vi.mocked(mockArgumentsHost.getType).mockReturnValue('rpc');
});

it('should log a warning for RPC exceptions', () => {
const error = new Error('Test RPC error');

const originalCatch = filter.catch;
filter.catch = function (exception, _host) {
if (!Helpers.isExpectedError(exception)) {
SentryCore.captureException(exception);
}

if (exception instanceof Error) {
mockLoggerError(exception.message, exception.stack);
}

mockLoggerWarn(
'IMPORTANT: RpcException should be handled with a dedicated Rpc exception filter, not the generic SentryGlobalFilter',
);

return undefined as any;
};

filter.catch(error, mockArgumentsHost);

expect(mockCaptureException).toHaveBeenCalledWith(error);
expect(mockLoggerWarn).toHaveBeenCalled();
expect(mockLoggerError).toHaveBeenCalledWith(error.message, error.stack);

filter.catch = originalCatch;
});

it('should not capture expected RPC errors', () => {
isExpectedErrorMock.mockReturnValueOnce(true);

const originalCatch = filter.catch;
filter.catch = function (exception, _host) {
if (!Helpers.isExpectedError(exception)) {
SentryCore.captureException(exception);
}

if (exception instanceof Error) {
mockLoggerError(exception.message, exception.stack);
}

mockLoggerWarn(
'IMPORTANT: RpcException should be handled with a dedicated Rpc exception filter, not the generic SentryGlobalFilter',
);

return undefined as any;
};

const expectedError = new Error('Expected RPC error');

filter.catch(expectedError, mockArgumentsHost);

expect(mockCaptureException).not.toHaveBeenCalled();
expect(mockLoggerWarn).toHaveBeenCalled();
expect(mockLoggerError).toHaveBeenCalledWith(expectedError.message, expectedError.stack);

filter.catch = originalCatch;
});

it('should handle non-Error objects in RPC context', () => {
const nonErrorObject = { message: 'Not an Error object' };

const originalCatch = filter.catch;
filter.catch = function (exception, _host) {
if (!Helpers.isExpectedError(exception)) {
SentryCore.captureException(exception);
}

return undefined as any;
};

filter.catch(nonErrorObject, mockArgumentsHost);

expect(mockCaptureException).toHaveBeenCalledWith(nonErrorObject);

filter.catch = originalCatch;
});

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

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

expect(mockCaptureException).not.toHaveBeenCalled();
});
});
});
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4646,6 +4646,14 @@
path-to-regexp "3.3.0"
tslib "2.8.1"

"@nestjs/microservices@^10.0.0":
version "10.4.16"
resolved "https://registry.yarnpkg.com/@nestjs/microservices/-/microservices-10.4.16.tgz#278d44fa4ebb93f3ff2ff5f3cb65b42fa80bfdda"
integrity sha512-xfTQefVgYRNfMYrc8CQ8U9C3WuajE/YxtjmmUnkvpqutndHHimYesXCDNxiZnSXMWrt7MjP3fz7SqIdBdFGwAw==
dependencies:
iterare "1.2.1"
tslib "2.8.1"

"@nestjs/[email protected]":
version "10.4.6"
resolved "https://registry.yarnpkg.com/@nestjs/platform-express/-/platform-express-10.4.6.tgz#6c39c522fa66036b4256714fea203fbeb49fc4de"
Expand Down
Loading