From 95f6c3c9d6630ec32c26c96ef22c3fc28e70fd96 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 15 Apr 2025 12:01:18 +0200 Subject: [PATCH 1/7] add SentryRpcFilter --- packages/nestjs/package.json | 19 ++- packages/nestjs/src/microservices.ts | 53 +++++++++ packages/nestjs/src/setup.ts | 12 +- packages/nestjs/test/microservices.test.ts | 128 +++++++++++++++++++++ yarn.lock | 19 ++- 5 files changed, 223 insertions(+), 8 deletions(-) create mode 100644 packages/nestjs/src/microservices.ts create mode 100644 packages/nestjs/test/microservices.test.ts diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index 86302ba66d2a..fb5530b626c2 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -38,6 +38,16 @@ "types": "./setup.d.ts", "default": "./build/cjs/setup.js" } + }, + "./microservices": { + "import": { + "types": "./microservices.d.ts", + "default": "./build/esm/microservices.js" + }, + "require": { + "types": "./microservices.d.ts", + "default": "./build/cjs/microservices.js" + } } }, "publishConfig": { @@ -55,11 +65,18 @@ "devDependencies": { "@nestjs/common": "^10.0.0", "@nestjs/core": "^10.0.0", + "@nestjs/microservices": "^10.0.0", "reflect-metadata": "^0.2.2" }, "peerDependencies": { "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0", - "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0" + "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0", + "@nestjs/microservices": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0" + }, + "peerDependenciesMeta": { + "@nestjs/microservices": { + "optional": true + } }, "scripts": { "build": "run-p build:transpile build:types", diff --git a/packages/nestjs/src/microservices.ts b/packages/nestjs/src/microservices.ts new file mode 100644 index 000000000000..31c235fe83d1 --- /dev/null +++ b/packages/nestjs/src/microservices.ts @@ -0,0 +1,53 @@ +import type { ArgumentsHost, HttpServer } from '@nestjs/common'; +import { Catch, Logger } from '@nestjs/common'; +import { RpcException } from '@nestjs/microservices'; +import { captureException } from '@sentry/core'; +import { SentryGlobalFilter } from './setup'; +import { isExpectedError } from './helpers'; + +/** + * Global filter to handle exceptions and report them to Sentry in nestjs microservice applications. + * Extends the standard SentryGlobalFilter with RPC exception handling. + */ +class SentryRpcFilter extends SentryGlobalFilter { + private readonly _rpcLogger: Logger; + + public constructor(applicationRef?: HttpServer) { + super(applicationRef); + this._rpcLogger = new Logger('RpcExceptionsHandler'); + } + + /** + * Extend the base filter with RPC-specific handling. + */ + public catch(exception: unknown, host: ArgumentsHost): void { + const contextType = host.getType(); + + if (contextType === 'rpc') { + // Don't report RpcExceptions as they are expected errors + if (exception instanceof RpcException) { + throw exception; + } + + if (!isExpectedError(exception)) { + if (exception instanceof Error) { + this._rpcLogger.error(exception.message, exception.stack); + } + captureException(exception); + } + + // Wrap non-RpcExceptions in RpcExceptions to avoid misleading error messages + if (!(exception instanceof RpcException)) { + const errorMessage = exception instanceof Error ? exception.message : 'Internal server error'; + throw new RpcException(errorMessage); + } + + throw exception; + } + + // For all other context types, use the base SentryGlobalFilter filter + return super.catch(exception, host); + } +} +Catch()(SentryRpcFilter); +export { SentryRpcFilter }; diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 045c196a0b8c..91c9a3b77c16 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,14 @@ class SentryGlobalFilter extends BaseExceptionFilter { throw exception; } + // Skip RPC contexts - they should be handled by SentryRpcFilter if the user has included it + if (contextType === 'rpc') { + // Let it propagate to a properly configured RPC filter if present + // Else it will be handled by the default NestJS exception system + throw exception; + } + + // HTTP exceptions if (!isExpectedError(exception)) { captureException(exception); } diff --git a/packages/nestjs/test/microservices.test.ts b/packages/nestjs/test/microservices.test.ts new file mode 100644 index 000000000000..ce4b414d013e --- /dev/null +++ b/packages/nestjs/test/microservices.test.ts @@ -0,0 +1,128 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import type { ArgumentsHost } from '@nestjs/common'; +import { RpcException } from '@nestjs/microservices'; +import { SentryRpcFilter } from '../src/microservices'; +import * as sentryCore from '@sentry/core'; + +vi.mock('@sentry/core', () => ({ + captureException: vi.fn(), + logger: { + warn: vi.fn(), + }, +})); + +describe('SentryRpcFilter', () => { + let filter: SentryRpcFilter; + let mockHost: ArgumentsHost; + + beforeEach(() => { + filter = new SentryRpcFilter(); + + mockHost = { + getType: vi.fn().mockReturnValue('rpc'), + switchToRpc: vi.fn().mockReturnValue({ + getData: vi.fn(), + getContext: vi.fn(), + }), + } as unknown as ArgumentsHost; + + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.resetAllMocks(); + }); + + it('should not report RpcException to Sentry', () => { + const rpcException = new RpcException('Expected RPC error'); + + expect(() => { + filter.catch(rpcException, mockHost); + }).toThrow(RpcException); + + expect(sentryCore.captureException).not.toHaveBeenCalled(); + }); + + it('should report regular Error to Sentry and wrap it in RpcException', () => { + const error = new Error('Unexpected error'); + + expect(() => { + filter.catch(error, mockHost); + }).toThrow(RpcException); + + expect(sentryCore.captureException).toHaveBeenCalledWith(error); + + try { + filter.catch(error, mockHost); + } catch (e) { + expect(e).toBeInstanceOf(RpcException); + expect(e.message).toContain('Unexpected error'); + } + }); + + it('should wrap string exceptions in RpcException', () => { + const errorMessage = 'String error message'; + + expect(() => { + filter.catch(errorMessage, mockHost); + }).toThrow(RpcException); + + expect(sentryCore.captureException).toHaveBeenCalledWith(errorMessage); + }); + + it('should handle null/undefined exceptions', () => { + expect(() => { + filter.catch(null, mockHost); + }).toThrow(RpcException); + + expect(sentryCore.captureException).toHaveBeenCalledWith(null); + + try { + filter.catch(null, mockHost); + } catch (e) { + expect(e).toBeInstanceOf(RpcException); + expect(e.message).toContain('Internal server error'); + } + }); + + it('should preserve the stack trace when possible', () => { + const originalError = new Error('Original error'); + originalError.stack = 'Original stack trace'; + + try { + filter.catch(originalError, mockHost); + } catch (e) { + expect(e).toBeInstanceOf(RpcException); + + // Extract the error inside the RpcException + const wrappedError = (e as any).getError(); + + // If implementation preserves stack, verify it + if (typeof wrappedError === 'object' && wrappedError.stack) { + expect(wrappedError.stack).toContain('Original stack trace'); + } + } + }); + + it('should properly handle non-rpc context by delegating to parent', () => { + // Mock HTTP context + const httpHost = { + getType: vi.fn().mockReturnValue('http'), + switchToHttp: vi.fn().mockReturnValue({ + getRequest: vi.fn(), + getResponse: vi.fn(), + }), + } as unknown as ArgumentsHost; + + // Mock the parent class behavior + const parentCatchSpy = vi.spyOn(filter, 'catch'); + parentCatchSpy.mockImplementation(vi.fn()); + + const error = new Error('HTTP error'); + + filter.catch(error, httpHost); + + // Verify parent catch was called + expect(parentCatchSpy).toHaveBeenCalled(); + }); +}); diff --git a/yarn.lock b/yarn.lock index f20d8694c742..8cbabeb22437 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4613,14 +4613,14 @@ dependencies: sparse-bitfield "^3.0.3" -"@nestjs/common@10.4.6": - version "10.4.6" - resolved "https://registry.yarnpkg.com/@nestjs/common/-/common-10.4.6.tgz#952e8fd0ceafeffcc4eaf47effd67fb395844ae0" - integrity sha512-KkezkZvU9poWaNq4L+lNvx+386hpOxPJkfXBBeSMrcqBOx8kVr36TGN2uYkF4Ta4zNu1KbCjmZbc0rhHSg296g== +"@nestjs/common@11.0.16": + version "11.0.16" + resolved "https://registry.yarnpkg.com/@nestjs/common/-/common-11.0.16.tgz#b6550ac2998e9991f24a99563a93475542885ba7" + integrity sha512-agvuQ8su4aZ+PVxAmY89odG1eR97HEQvxPmTMdDqyvDWzNerl7WQhUEd+j4/UyNWcF1or1UVcrtPj52x+eUSsA== dependencies: uid "2.0.2" iterare "1.2.1" - tslib "2.7.0" + tslib "2.8.1" "@nestjs/common@^10.0.0": version "10.4.15" @@ -4655,6 +4655,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" @@ -27039,7 +27047,6 @@ stylus@0.59.0, stylus@^0.59.0: sucrase@^3.27.0, sucrase@^3.35.0, sucrase@getsentry/sucrase#es2020-polyfills: version "3.36.0" - uid fd682f6129e507c00bb4e6319cc5d6b767e36061 resolved "https://codeload.github.com/getsentry/sucrase/tar.gz/fd682f6129e507c00bb4e6319cc5d6b767e36061" dependencies: "@jridgewell/gen-mapping" "^0.3.2" From e9d52ea79040dfd09e2fd92ad7c021caf1b5b5d0 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 16 Apr 2025 13:46:55 +0200 Subject: [PATCH 2/7] add jsdoc --- packages/nestjs/src/microservices.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/nestjs/src/microservices.ts b/packages/nestjs/src/microservices.ts index 31c235fe83d1..cd9f5009a4af 100644 --- a/packages/nestjs/src/microservices.ts +++ b/packages/nestjs/src/microservices.ts @@ -8,6 +8,24 @@ import { isExpectedError } from './helpers'; /** * Global filter to handle exceptions and report them to Sentry in nestjs microservice applications. * Extends the standard SentryGlobalFilter with RPC exception handling. + * + * @example + * ``` + * import { Module } from '@nestjs/common'; + * import { APP_FILTER } from '@nestjs/core'; + * import { SentryRpcFilter } from '@sentry/nestjs/microservices'; + * + * @Module({ + * providers: [ + * // For microservice applications this filter replaces SentryGlobalFilter + * { + * provide: APP_FILTER, + * useClass: SentryRpcFilter, + * }, + * ], + * }) + * export class AppModule {} + * ``` */ class SentryRpcFilter extends SentryGlobalFilter { private readonly _rpcLogger: Logger; From c197522ceb943ef352842fdae549d54b086ab557 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 16 Apr 2025 14:04:21 +0200 Subject: [PATCH 3/7] add rollup entrypoint --- packages/nestjs/rollup.npm.config.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nestjs/rollup.npm.config.mjs b/packages/nestjs/rollup.npm.config.mjs index 0ce71546935c..f5bb172ec811 100644 --- a/packages/nestjs/rollup.npm.config.mjs +++ b/packages/nestjs/rollup.npm.config.mjs @@ -2,6 +2,6 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollu export default makeNPMConfigVariants( makeBaseNPMConfig({ - entrypoints: ['src/index.ts', 'src/setup.ts'], + entrypoints: ['src/index.ts', 'src/setup.ts', 'src/microservices.ts'], }), ); From cdf559d7095e9bec9a05b1a05a8a49f54b4d8535 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Thu, 17 Apr 2025 12:05:24 +0200 Subject: [PATCH 4/7] revert microservice handling --- packages/nestjs/rollup.npm.config.mjs | 2 +- packages/nestjs/src/microservices.ts | 71 ------ packages/nestjs/src/setup.ts | 33 ++- packages/nestjs/test/microservices.test.ts | 128 ---------- .../nestjs/test/sentry-global-filter.test.ts | 235 ++++++++++++++++++ 5 files changed, 265 insertions(+), 204 deletions(-) delete mode 100644 packages/nestjs/src/microservices.ts delete mode 100644 packages/nestjs/test/microservices.test.ts create mode 100644 packages/nestjs/test/sentry-global-filter.test.ts diff --git a/packages/nestjs/rollup.npm.config.mjs b/packages/nestjs/rollup.npm.config.mjs index f5bb172ec811..0ce71546935c 100644 --- a/packages/nestjs/rollup.npm.config.mjs +++ b/packages/nestjs/rollup.npm.config.mjs @@ -2,6 +2,6 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollu export default makeNPMConfigVariants( makeBaseNPMConfig({ - entrypoints: ['src/index.ts', 'src/setup.ts', 'src/microservices.ts'], + entrypoints: ['src/index.ts', 'src/setup.ts'], }), ); diff --git a/packages/nestjs/src/microservices.ts b/packages/nestjs/src/microservices.ts deleted file mode 100644 index cd9f5009a4af..000000000000 --- a/packages/nestjs/src/microservices.ts +++ /dev/null @@ -1,71 +0,0 @@ -import type { ArgumentsHost, HttpServer } from '@nestjs/common'; -import { Catch, Logger } from '@nestjs/common'; -import { RpcException } from '@nestjs/microservices'; -import { captureException } from '@sentry/core'; -import { SentryGlobalFilter } from './setup'; -import { isExpectedError } from './helpers'; - -/** - * Global filter to handle exceptions and report them to Sentry in nestjs microservice applications. - * Extends the standard SentryGlobalFilter with RPC exception handling. - * - * @example - * ``` - * import { Module } from '@nestjs/common'; - * import { APP_FILTER } from '@nestjs/core'; - * import { SentryRpcFilter } from '@sentry/nestjs/microservices'; - * - * @Module({ - * providers: [ - * // For microservice applications this filter replaces SentryGlobalFilter - * { - * provide: APP_FILTER, - * useClass: SentryRpcFilter, - * }, - * ], - * }) - * export class AppModule {} - * ``` - */ -class SentryRpcFilter extends SentryGlobalFilter { - private readonly _rpcLogger: Logger; - - public constructor(applicationRef?: HttpServer) { - super(applicationRef); - this._rpcLogger = new Logger('RpcExceptionsHandler'); - } - - /** - * Extend the base filter with RPC-specific handling. - */ - public catch(exception: unknown, host: ArgumentsHost): void { - const contextType = host.getType(); - - if (contextType === 'rpc') { - // Don't report RpcExceptions as they are expected errors - if (exception instanceof RpcException) { - throw exception; - } - - if (!isExpectedError(exception)) { - if (exception instanceof Error) { - this._rpcLogger.error(exception.message, exception.stack); - } - captureException(exception); - } - - // Wrap non-RpcExceptions in RpcExceptions to avoid misleading error messages - if (!(exception instanceof RpcException)) { - const errorMessage = exception instanceof Error ? exception.message : 'Internal server error'; - throw new RpcException(errorMessage); - } - - throw exception; - } - - // For all other context types, use the base SentryGlobalFilter filter - return super.catch(exception, host); - } -} -Catch()(SentryRpcFilter); -export { SentryRpcFilter }; diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index 91c9a3b77c16..13d44e74a204 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -105,11 +105,36 @@ class SentryGlobalFilter extends BaseExceptionFilter { throw exception; } - // Skip RPC contexts - they should be handled by SentryRpcFilter if the user has included it + // 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') { - // Let it propagate to a properly configured RPC filter if present - // Else it will be handled by the default NestJS exception system - throw exception; + // 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 diff --git a/packages/nestjs/test/microservices.test.ts b/packages/nestjs/test/microservices.test.ts deleted file mode 100644 index ce4b414d013e..000000000000 --- a/packages/nestjs/test/microservices.test.ts +++ /dev/null @@ -1,128 +0,0 @@ -import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; -import type { ArgumentsHost } from '@nestjs/common'; -import { RpcException } from '@nestjs/microservices'; -import { SentryRpcFilter } from '../src/microservices'; -import * as sentryCore from '@sentry/core'; - -vi.mock('@sentry/core', () => ({ - captureException: vi.fn(), - logger: { - warn: vi.fn(), - }, -})); - -describe('SentryRpcFilter', () => { - let filter: SentryRpcFilter; - let mockHost: ArgumentsHost; - - beforeEach(() => { - filter = new SentryRpcFilter(); - - mockHost = { - getType: vi.fn().mockReturnValue('rpc'), - switchToRpc: vi.fn().mockReturnValue({ - getData: vi.fn(), - getContext: vi.fn(), - }), - } as unknown as ArgumentsHost; - - vi.clearAllMocks(); - }); - - afterEach(() => { - vi.resetAllMocks(); - }); - - it('should not report RpcException to Sentry', () => { - const rpcException = new RpcException('Expected RPC error'); - - expect(() => { - filter.catch(rpcException, mockHost); - }).toThrow(RpcException); - - expect(sentryCore.captureException).not.toHaveBeenCalled(); - }); - - it('should report regular Error to Sentry and wrap it in RpcException', () => { - const error = new Error('Unexpected error'); - - expect(() => { - filter.catch(error, mockHost); - }).toThrow(RpcException); - - expect(sentryCore.captureException).toHaveBeenCalledWith(error); - - try { - filter.catch(error, mockHost); - } catch (e) { - expect(e).toBeInstanceOf(RpcException); - expect(e.message).toContain('Unexpected error'); - } - }); - - it('should wrap string exceptions in RpcException', () => { - const errorMessage = 'String error message'; - - expect(() => { - filter.catch(errorMessage, mockHost); - }).toThrow(RpcException); - - expect(sentryCore.captureException).toHaveBeenCalledWith(errorMessage); - }); - - it('should handle null/undefined exceptions', () => { - expect(() => { - filter.catch(null, mockHost); - }).toThrow(RpcException); - - expect(sentryCore.captureException).toHaveBeenCalledWith(null); - - try { - filter.catch(null, mockHost); - } catch (e) { - expect(e).toBeInstanceOf(RpcException); - expect(e.message).toContain('Internal server error'); - } - }); - - it('should preserve the stack trace when possible', () => { - const originalError = new Error('Original error'); - originalError.stack = 'Original stack trace'; - - try { - filter.catch(originalError, mockHost); - } catch (e) { - expect(e).toBeInstanceOf(RpcException); - - // Extract the error inside the RpcException - const wrappedError = (e as any).getError(); - - // If implementation preserves stack, verify it - if (typeof wrappedError === 'object' && wrappedError.stack) { - expect(wrappedError.stack).toContain('Original stack trace'); - } - } - }); - - it('should properly handle non-rpc context by delegating to parent', () => { - // Mock HTTP context - const httpHost = { - getType: vi.fn().mockReturnValue('http'), - switchToHttp: vi.fn().mockReturnValue({ - getRequest: vi.fn(), - getResponse: vi.fn(), - }), - } as unknown as ArgumentsHost; - - // Mock the parent class behavior - const parentCatchSpy = vi.spyOn(filter, 'catch'); - parentCatchSpy.mockImplementation(vi.fn()); - - const error = new Error('HTTP error'); - - filter.catch(error, httpHost); - - // Verify parent catch was called - expect(parentCatchSpy).toHaveBeenCalled(); - }); -}); 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(); + }); + }); +}); From e4fd6f1443ae64e51b37399e771ede16c344dfb4 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Thu, 17 Apr 2025 12:22:32 +0200 Subject: [PATCH 5/7] rm npm export --- packages/nestjs/package.json | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index fb5530b626c2..297e35442869 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -38,16 +38,6 @@ "types": "./setup.d.ts", "default": "./build/cjs/setup.js" } - }, - "./microservices": { - "import": { - "types": "./microservices.d.ts", - "default": "./build/esm/microservices.js" - }, - "require": { - "types": "./microservices.d.ts", - "default": "./build/cjs/microservices.js" - } } }, "publishConfig": { From 4c1cb112e6517eee84af74433adc051827ce4687 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 22 Apr 2025 15:18:14 +0200 Subject: [PATCH 6/7] remove peer --- packages/nestjs/package.json | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index 297e35442869..d26a63bf8922 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -60,13 +60,7 @@ }, "peerDependencies": { "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0", - "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0", - "@nestjs/microservices": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0" - }, - "peerDependenciesMeta": { - "@nestjs/microservices": { - "optional": true - } + "@nestjs/core": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0" }, "scripts": { "build": "run-p build:transpile build:types", From 3b5ca3f2c19ff5738435fce0967fcb3be0715e8c Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 23 Apr 2025 09:28:25 +0200 Subject: [PATCH 7/7] remove dev dep --- packages/nestjs/package.json | 1 - yarn.lock | 8 -------- 2 files changed, 9 deletions(-) diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index 8993f8f4b97d..2bac9c6816ec 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -55,7 +55,6 @@ "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/yarn.lock b/yarn.lock index 31e9ce5134ac..8895daad18dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4646,14 +4646,6 @@ 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"