Skip to content

ref(nestjs): Add mechanism to captured errors #17312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
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
Expand Up @@ -71,6 +71,11 @@ test('Sends exceptions to Sentry on error in cron job', async ({ baseURL }) => {

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('Test error from cron job');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.cron.nestjs.async',
});

expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
span_id: expect.stringMatching(/[a-f0-9]{16}/),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ test('Sends exception to Sentry', async ({ baseURL }) => {

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ test('Sends exception to Sentry', async ({ baseURL }) => {

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down Expand Up @@ -102,6 +107,11 @@ test('Sends graphql exception to Sentry', async ({ baseURL }) => {
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.graphql.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'POST',
cookies: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ test('Sends exceptions to Sentry on error in async cron job', async ({ baseURL }
span_id: expect.stringMatching(/[a-f0-9]{16}/),
});

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.cron.nestjs.async',
});

// kill cron so tests don't get stuck
await fetch(`${baseURL}/kill-test-cron/test-async-cron-error`);
});
Expand All @@ -92,6 +97,11 @@ test('Sends exceptions to Sentry on error in sync cron job', async ({ baseURL })
span_id: expect.stringMatching(/[a-f0-9]{16}/),
});

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.cron.nestjs',
});

// kill cron so tests don't get stuck
await fetch(`${baseURL}/kill-test-cron/test-sync-cron-error`);
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ test('Sends exception to Sentry', async ({ baseURL }) => {

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ test('Event emitter', async () => {
type: 'Error',
value: 'Test error from event handler',
stacktrace: expect.any(Object),
mechanism: expect.any(Object),
mechanism: {
handled: false,
type: 'auto.event.nestjs',
},
},
],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ test('Sends exceptions to Sentry on error in cron job', async ({ baseURL }) => {

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('Test error from cron job');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.cron.nestjs.async',
});
expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
span_id: expect.stringMatching(/[a-f0-9]{16}/),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.graphql.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'POST',
cookies: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.function.nestjs.exception_captured',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down Expand Up @@ -59,6 +64,11 @@ test('Sends unexpected exception to Sentry if thrown in module with local filter
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.function.nestjs.exception_captured',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down Expand Up @@ -98,6 +108,11 @@ test('Sends unexpected exception to Sentry if thrown in module that was register
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.function.nestjs.exception_captured',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ test('Sends unexpected exception to Sentry if thrown in module with global filte
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down Expand Up @@ -43,6 +48,11 @@ test('Sends unexpected exception to Sentry if thrown in module with local filter
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down Expand Up @@ -74,6 +84,11 @@ test('Sends unexpected exception to Sentry if thrown in module that was register
expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an uncaught exception!');

expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'auto.http.nestjs.global_filter',
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
Expand Down
6 changes: 3 additions & 3 deletions packages/nestjs/src/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export const SentryCron = (monitorSlug: string, monitorConfig?: MonitorConfig):
try {
result = originalMethod.apply(this, args);
} catch (e) {
captureException(e);
captureException(e, { mechanism: { handled: false, type: 'auto.cron.nestjs' } });
Copy link
Member Author

@Lms24 Lms24 Aug 5, 2025

Choose a reason for hiding this comment

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

(auto.)cron is not a listed category, which is a bit suboptimal. The two alternatives are:

  • function given we wrap some kind of cron-executed function
  • omitting the category, since it's not required: auto.nestjs.sentry_cron

@AbhiPrasad thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

worth noting: I didn't find a specific span that reliably applies here, so there's no direct span counterpart to this mechanism.type

Copy link
Member

Choose a reason for hiding this comment

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

let's do auto.function.X

throw e;
}
if (isThenable(result)) {
return result.then(undefined, e => {
captureException(e);
captureException(e, { mechanism: { handled: false, type: 'auto.cron.nestjs.async' } });
throw e;
});
}
Expand Down Expand Up @@ -77,7 +77,7 @@ export function SentryExceptionCaptured() {
return originalCatch.apply(this, [exception, host, ...args]);
}

captureException(exception);
captureException(exception, { mechanism: { handled: false, type: 'auto.function.nestjs.exception_captured' } });
Copy link
Member Author

Choose a reason for hiding this comment

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

I think auto.function.* is correct here, given this decorator can be applied to any arbitrary function.

return originalCatch.apply(this, [exception, host, ...args]);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,12 @@ export class SentryNestEventInstrumentation extends InstrumentationBase {
return result;
} catch (error) {
// exceptions from event handlers are not caught by global error filter
captureException(error);
captureException(error, {
mechanism: {
handled: false,
type: 'auto.event.nestjs',
},
});
throw error;
}
});
Expand Down
28 changes: 24 additions & 4 deletions packages/nestjs/src/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ class SentryGlobalFilter extends BaseExceptionFilter {
this._logger.error(exception.message, exception.stack);
}

captureException(exception);
captureException(exception, {
mechanism: {
handled: false,
type: 'auto.graphql.nestjs.global_filter',
},
});
throw exception;
}

Expand All @@ -117,15 +122,25 @@ class SentryGlobalFilter extends BaseExceptionFilter {
// Handle any other kind of error
if (!(exception instanceof Error)) {
if (!isExpectedError(exception)) {
captureException(exception);
captureException(exception, {
mechanism: {
handled: false,
type: 'auto.rpc.nestjs.global_filter',
},
});
}
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);
captureException(exception, {
mechanism: {
handled: false,
type: 'auto.rpc.nestjs.global_filter',
},
});
}

this._logger.warn(
Expand All @@ -139,7 +154,12 @@ class SentryGlobalFilter extends BaseExceptionFilter {

// HTTP exceptions
if (!isExpectedError(exception)) {
captureException(exception);
captureException(exception, {
mechanism: {
handled: false,
type: 'auto.http.nestjs.global_filter',
},
});
}

return super.catch(exception, host);
Expand Down
7 changes: 6 additions & 1 deletion packages/nestjs/test/decorators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,12 @@ describe('SentryExceptionCaptured decorator', () => {
decoratedMethod(exception, host);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
expect(captureExceptionSpy).toHaveBeenCalledWith(exception);
expect(captureExceptionSpy).toHaveBeenCalledWith(exception, {
mechanism: {
handled: false,
type: 'auto.function.nestjs.exception_captured',
},
});
expect(originalCatch).toHaveBeenCalledWith(exception, host);

isExpectedErrorSpy.mockRestore();
Expand Down
7 changes: 6 additions & 1 deletion packages/nestjs/test/integrations/nest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,12 @@ describe('Nest', () => {
decorated(mockTarget, 'testMethod', descriptor);

await expect(descriptor.value()).rejects.toThrow(error);
expect(core.captureException).toHaveBeenCalledWith(error);
expect(core.captureException).toHaveBeenCalledWith(error, {
mechanism: {
handled: false,
type: 'auto.event.nestjs',
},
});
});

it('should skip wrapping for internal Sentry handlers', () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/nestjs/test/sentry-global-filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,12 @@ describe('SentryGlobalFilter', () => {
filter.catch(error, mockArgumentsHost);
}).toThrow(error);

expect(mockCaptureException).toHaveBeenCalledWith(error);
expect(mockCaptureException).toHaveBeenCalledWith(error, {
mechanism: {
handled: false,
type: 'auto.graphql.nestjs.global_filter',
},
});
expect(mockLoggerError).toHaveBeenCalledWith(error.message, error.stack);
});
});
Expand Down
Loading