From 5f4fda67ab8dbf9ce8ddde6aebc5b9cafaa62ddf Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 5 Mar 2025 16:32:26 +0100 Subject: [PATCH 1/2] update nest decorators --- packages/nestjs/package.json | 3 +- packages/nestjs/src/decorators.ts | 47 ++- packages/nestjs/test/decorators.test.ts | 376 ++++++++++++++++++++++++ yarn.lock | 5 + 4 files changed, 422 insertions(+), 9 deletions(-) create mode 100644 packages/nestjs/test/decorators.test.ts diff --git a/packages/nestjs/package.json b/packages/nestjs/package.json index efdff9b0edaf..b363e6a51b2e 100644 --- a/packages/nestjs/package.json +++ b/packages/nestjs/package.json @@ -54,7 +54,8 @@ }, "devDependencies": { "@nestjs/common": "^10.0.0", - "@nestjs/core": "^10.0.0" + "@nestjs/core": "^10.0.0", + "reflect-metadata": "^0.2.2" }, "peerDependencies": { "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0", diff --git a/packages/nestjs/src/decorators.ts b/packages/nestjs/src/decorators.ts index 42592e62d553..aa0270a229e1 100644 --- a/packages/nestjs/src/decorators.ts +++ b/packages/nestjs/src/decorators.ts @@ -20,6 +20,9 @@ export const SentryCron = (monitorSlug: string, monitorConfig?: MonitorConfig): monitorConfig, ); }; + + copyFunctionNameAndMetadata({ originalMethod, descriptor }); + return descriptor; }; }; @@ -28,7 +31,7 @@ export const SentryCron = (monitorSlug: string, monitorConfig?: MonitorConfig): * A decorator usable to wrap arbitrary functions with spans. */ export function SentryTraced(op: string = 'function') { - return function (target: unknown, propertyKey: string, descriptor: PropertyDescriptor) { + return function (_target: unknown, propertyKey: string, descriptor: PropertyDescriptor) { const originalMethod = descriptor.value as (...args: unknown[]) => Promise | unknown; // function can be sync or async descriptor.value = function (...args: unknown[]) { @@ -43,13 +46,7 @@ export function SentryTraced(op: string = 'function') { ); }; - // preserve the original name on the decorated function - Object.defineProperty(descriptor.value, 'name', { - value: originalMethod.name, - configurable: true, - enumerable: true, - writable: true, - }); + copyFunctionNameAndMetadata({ originalMethod, descriptor }); return descriptor; }; @@ -71,6 +68,40 @@ export function SentryExceptionCaptured() { return originalCatch.apply(this, [exception, host, ...args]); }; + copyFunctionNameAndMetadata({ originalMethod: originalCatch, descriptor }); + return descriptor; }; } + +/** + * Copies the function name and metadata from the original method to the decorated method. + * This ensures that the decorated method maintains the same name and metadata as the original. + * + * @param {Function} params.originalMethod - The original method being decorated + * @param {PropertyDescriptor} params.descriptor - The property descriptor containing the decorated method + */ +function copyFunctionNameAndMetadata({ + originalMethod, + descriptor, +}: { + originalMethod: (...args: unknown[]) => Promise | unknown; + descriptor: PropertyDescriptor; +}): void { + // preserve the original name on the decorated function + Object.defineProperty(descriptor.value, 'name', { + value: originalMethod.name, + configurable: true, + enumerable: true, + writable: true, + }); + + // copy metadata + if (typeof Reflect !== 'undefined' && typeof Reflect.getMetadataKeys === 'function') { + const originalMetaData = Reflect.getMetadataKeys(originalMethod); + for (const key of originalMetaData) { + const value = Reflect.getMetadata(key, originalMethod); + Reflect.defineMetadata(key, value, descriptor.value); + } + } +} diff --git a/packages/nestjs/test/decorators.test.ts b/packages/nestjs/test/decorators.test.ts new file mode 100644 index 000000000000..e3e99b700273 --- /dev/null +++ b/packages/nestjs/test/decorators.test.ts @@ -0,0 +1,376 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { SentryCron, SentryExceptionCaptured, SentryTraced } from '../src/decorators'; +import * as core from '@sentry/core'; +import * as helpers from '../src/helpers'; +import 'reflect-metadata'; + +describe('SentryTraced decorator', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should create a span with correct operation and name', async () => { + const startSpanSpy = vi.spyOn(core, 'startSpan'); + + const originalMethod = async (param1: string, param2: number): Promise => { + return `${param1}-${param2}`; + }; + + const descriptor: PropertyDescriptor = { + value: originalMethod, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryTraced('test-operation')( + {}, // target + 'testMethod', + descriptor, + ); + + const decoratedMethod = decoratedDescriptor.value as typeof originalMethod; + const result = await decoratedMethod('test', 123); + + expect(result).toBe('test-123'); + expect(startSpanSpy).toHaveBeenCalledTimes(1); + expect(startSpanSpy).toHaveBeenCalledWith( + { + op: 'test-operation', + name: 'testMethod', + }, + expect.any(Function), + ); + }); + + it('should use default operation name when not provided', async () => { + const startSpanSpy = vi.spyOn(core, 'startSpan'); + + const originalMethod = async (): Promise => { + return 'success'; + }; + + const descriptor: PropertyDescriptor = { + value: originalMethod, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryTraced()({}, 'testDefaultOp', descriptor); + const decoratedMethod = decoratedDescriptor.value as typeof originalMethod; + const result = await decoratedMethod(); + + expect(result).toBe('success'); + expect(startSpanSpy).toHaveBeenCalledTimes(1); + expect(startSpanSpy).toHaveBeenCalledWith( + { + op: 'function', // default value + name: 'testDefaultOp', + }, + expect.any(Function), + ); + }); + + it('should work with synchronous methods', () => { + const startSpanSpy = vi.spyOn(core, 'startSpan'); + + const originalMethod = (value: number): number => { + return value * 2; + }; + + const descriptor: PropertyDescriptor = { + value: originalMethod, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryTraced('sync-operation')({}, 'syncMethod', descriptor); + const decoratedMethod = decoratedDescriptor.value as typeof originalMethod; + const result = decoratedMethod(5); + + expect(result).toBe(10); + expect(startSpanSpy).toHaveBeenCalledTimes(1); + expect(startSpanSpy).toHaveBeenCalledWith( + { + op: 'sync-operation', + name: 'syncMethod', + }, + expect.any(Function), + ); + }); + + it('should handle complex object parameters', () => { + const startSpanSpy = vi.spyOn(core, 'startSpan'); + + const originalMethod = (data: { id: number; items: string[] }): string[] => { + return data.items.map(item => `${data.id}-${item}`); + }; + + const descriptor: PropertyDescriptor = { + value: originalMethod, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryTraced('data-processing')({}, 'processData', descriptor); + const decoratedMethod = decoratedDescriptor.value as typeof originalMethod; + const complexData = { id: 123, items: ['a', 'b', 'c'] }; + const result = decoratedMethod(complexData); + + expect(result).toEqual(['123-a', '123-b', '123-c']); + expect(startSpanSpy).toHaveBeenCalledTimes(1); + }); + + it('should preserve function metadata', () => { + const getMetadataKeysSpy = vi.spyOn(Reflect, 'getMetadataKeys').mockReturnValue(['test-key']); + const getMetadataSpy = vi.spyOn(Reflect, 'getMetadata').mockReturnValue('test-value'); + const defineMetadataSpy = vi.spyOn(Reflect, 'defineMetadata').mockImplementation(() => {}); + + const originalMethod = () => 'result'; + const descriptor = { + value: originalMethod, + writable: true, + configurable: true, + enumerable: true, + }; + + const decoratedDescriptor = SentryTraced()({}, 'metadataMethod', descriptor); + decoratedDescriptor.value(); + + expect(getMetadataKeysSpy).toHaveBeenCalled(); + expect(getMetadataSpy).toHaveBeenCalled(); + expect(defineMetadataSpy).toHaveBeenCalled(); + + getMetadataKeysSpy.mockRestore(); + getMetadataSpy.mockRestore(); + defineMetadataSpy.mockRestore(); + }); +}); + +describe('SentryCron decorator', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should call withMonitor with correct parameters', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + + const originalMethod = async (): Promise => { + return 'success'; + }; + + const descriptor: PropertyDescriptor = { + value: originalMethod, + writable: true, + enumerable: true, + configurable: true, + }; + + const monitorSlug = 'test-monitor'; + const monitorConfig: core.MonitorConfig = { schedule: { value: '10 * * * *', type: 'crontab' } }; + + const decoratedDescriptor = SentryCron(monitorSlug, monitorConfig)( + {}, // target + 'cronMethod', + descriptor, + ); + + const decoratedMethod = decoratedDescriptor?.value as typeof originalMethod; + const result = await decoratedMethod(); + + expect(result).toBe('success'); + expect(withMonitorSpy).toHaveBeenCalledTimes(1); + expect(withMonitorSpy).toHaveBeenCalledWith(monitorSlug, expect.any(Function), monitorConfig); + }); + + it('should work with optional monitor config', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + + const originalMethod = async (): Promise => { + return 'success'; + }; + + const descriptor: PropertyDescriptor = { + value: originalMethod, + writable: true, + enumerable: true, + configurable: true, + }; + + const monitorSlug = 'test-monitor'; + + const decoratedDescriptor = SentryCron(monitorSlug)( + {}, // target + 'cronMethod', + descriptor, + ); + + const decoratedMethod = decoratedDescriptor?.value as typeof originalMethod; + const result = await decoratedMethod(); + + expect(result).toBe('success'); + expect(withMonitorSpy).toHaveBeenCalledTimes(1); + expect(withMonitorSpy).toHaveBeenCalledWith(monitorSlug, expect.any(Function), undefined); + }); + + it('should preserve function metadata', () => { + const getMetadataKeysSpy = vi.spyOn(Reflect, 'getMetadataKeys').mockReturnValue(['cron-key']); + const getMetadataSpy = vi.spyOn(Reflect, 'getMetadata').mockReturnValue('cron-value'); + const defineMetadataSpy = vi.spyOn(Reflect, 'defineMetadata').mockImplementation(() => {}); + + const originalMethod = () => 'cron result'; + const descriptor = { + value: originalMethod, + writable: true, + configurable: true, + enumerable: true, + }; + + const decoratedDescriptor = SentryCron('monitor-slug')({}, 'cronMethod', descriptor); + typeof decoratedDescriptor?.value === 'function' && decoratedDescriptor.value(); + + expect(getMetadataKeysSpy).toHaveBeenCalled(); + expect(getMetadataSpy).toHaveBeenCalled(); + expect(defineMetadataSpy).toHaveBeenCalled(); + + getMetadataKeysSpy.mockRestore(); + getMetadataSpy.mockRestore(); + defineMetadataSpy.mockRestore(); + }); +}); + +describe('SentryExceptionCaptured decorator', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should capture non-expected exceptions', () => { + const captureExceptionSpy = vi.spyOn(core, 'captureException'); + const isExpectedErrorSpy = vi.spyOn(helpers, 'isExpectedError').mockReturnValue(false); + + const originalCatch = vi.fn().mockImplementation((exception, host) => { + return { exception, host }; + }); + + const descriptor: PropertyDescriptor = { + value: originalCatch, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryExceptionCaptured()( + {}, // target + 'catch', + descriptor, + ); + + const decoratedMethod = decoratedDescriptor.value; + const exception = new Error('Test exception'); + const host = { switchToHttp: () => ({}) }; + + decoratedMethod(exception, host); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(exception); + expect(originalCatch).toHaveBeenCalledWith(exception, host); + + isExpectedErrorSpy.mockRestore(); + }); + + it('should not capture expected exceptions', () => { + const captureExceptionSpy = vi.spyOn(core, 'captureException'); + const isExpectedErrorSpy = vi.spyOn(helpers, 'isExpectedError').mockReturnValue(true); + + const originalCatch = vi.fn().mockImplementation((exception, host) => { + return { exception, host }; + }); + + const descriptor: PropertyDescriptor = { + value: originalCatch, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryExceptionCaptured()( + {}, // target + 'catch', + descriptor, + ); + + const decoratedMethod = decoratedDescriptor.value; + const exception = new Error('Expected exception'); + const host = { switchToHttp: () => ({}) }; + + decoratedMethod(exception, host); + + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(originalCatch).toHaveBeenCalledWith(exception, host); + + isExpectedErrorSpy.mockRestore(); + }); + + it('should preserve function metadata', () => { + const getMetadataKeysSpy = vi.spyOn(Reflect, 'getMetadataKeys').mockReturnValue(['exception-key']); + const getMetadataSpy = vi.spyOn(Reflect, 'getMetadata').mockReturnValue('exception-value'); + const defineMetadataSpy = vi.spyOn(Reflect, 'defineMetadata').mockImplementation(() => {}); + + const originalMethod = () => ({ handled: true }); + const descriptor = { + value: originalMethod, + writable: true, + configurable: true, + enumerable: true, + }; + + const decoratedDescriptor = SentryExceptionCaptured()({}, 'catch', descriptor); + vi.spyOn(helpers, 'isExpectedError').mockReturnValue(true); + + decoratedDescriptor.value(new Error(), {}); + + expect(getMetadataKeysSpy).toHaveBeenCalled(); + expect(getMetadataSpy).toHaveBeenCalled(); + expect(defineMetadataSpy).toHaveBeenCalled(); + + getMetadataKeysSpy.mockRestore(); + getMetadataSpy.mockRestore(); + defineMetadataSpy.mockRestore(); + }); + + it('should handle additional arguments', () => { + const captureExceptionSpy = vi.spyOn(core, 'captureException'); + vi.spyOn(helpers, 'isExpectedError').mockReturnValue(false); + + const originalCatch = vi.fn().mockImplementation((exception, host, arg1, arg2) => { + return { exception, host, arg1, arg2 }; + }); + + const descriptor: PropertyDescriptor = { + value: originalCatch, + writable: true, + enumerable: true, + configurable: true, + }; + + const decoratedDescriptor = SentryExceptionCaptured()( + {}, // target + 'catch', + descriptor, + ); + + const decoratedMethod = decoratedDescriptor.value; + const exception = new Error('Test exception'); + const host = { switchToHttp: () => ({}) }; + const arg1 = 'extra1'; + const arg2 = 'extra2'; + + decoratedMethod(exception, host, arg1, arg2); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(originalCatch).toHaveBeenCalledWith(exception, host, arg1, arg2); + }); +}); diff --git a/yarn.lock b/yarn.lock index b4f8c171dd4d..984ff3f3d7d1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26244,6 +26244,11 @@ reflect-metadata@^0.1.2: resolved "https://registry.yarnpkg.com/reflect-metadata/-/reflect-metadata-0.1.13.tgz#67ae3ca57c972a2aa1642b10fe363fe32d49dc08" integrity sha512-Ts1Y/anZELhSsjMcU605fU9RE4Oi3p5ORujwbIKXfWa+0Zxs510Qrmrce5/Jowq3cHSZSJqBjypxmHarc+vEWg== +reflect-metadata@^0.2.2: + version "0.2.2" + resolved "https://registry.yarnpkg.com/reflect-metadata/-/reflect-metadata-0.2.2.tgz#400c845b6cba87a21f2c65c4aeb158f4fa4d9c5b" + integrity sha512-urBwgfrvVP/eAyXx4hluJivBKzuEbSQs9rKWCrCkbSxNv8mxPcUZKeuoF3Uy4mJl3Lwprp6yy5/39VWigZ4K6Q== + regenerate-unicode-properties@^10.1.0: version "10.1.0" resolved "https://registry.yarnpkg.com/regenerate-unicode-properties/-/regenerate-unicode-properties-10.1.0.tgz#7c3192cab6dd24e21cb4461e5ddd7dd24fa8374c" From b127708e80072434b15ed8a13084ccb4f656abcb Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Wed, 5 Mar 2025 17:02:05 +0100 Subject: [PATCH 2/2] update return type --- packages/nestjs/src/decorators.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nestjs/src/decorators.ts b/packages/nestjs/src/decorators.ts index aa0270a229e1..69d54377332d 100644 --- a/packages/nestjs/src/decorators.ts +++ b/packages/nestjs/src/decorators.ts @@ -85,8 +85,8 @@ function copyFunctionNameAndMetadata({ originalMethod, descriptor, }: { - originalMethod: (...args: unknown[]) => Promise | unknown; descriptor: PropertyDescriptor; + originalMethod: (...args: unknown[]) => unknown; }): void { // preserve the original name on the decorated function Object.defineProperty(descriptor.value, 'name', {