diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index 2bac9c6816ec..8993f8f4b97d 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -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": { diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 045c196a0b8c..13d44e74a204 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -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(); + // 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; @@ -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); } diff --git a/packages/nestjs/test/sentry-global-filter.test.ts b/packages/nestjs/test/sentry-global-filter.test.ts new file mode 100644 index 000000000000..f144e9fad8ec --- /dev/null +++ b/packages/nestjs/test/sentry-global-filter.test.ts @@ -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(); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 8895daad18dc..31e9ce5134ac 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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/platform-express@10.4.6": version "10.4.6" resolved "https://registry.yarnpkg.com/@nestjs/platform-express/-/platform-express-10.4.6.tgz#6c39c522fa66036b4256714fea203fbeb49fc4de"