diff --git a/dev-packages/e2e-tests/run.ts b/dev-packages/e2e-tests/run.ts index e8901eede1b9..44f0bc06dca7 100644 --- a/dev-packages/e2e-tests/run.ts +++ b/dev-packages/e2e-tests/run.ts @@ -66,6 +66,7 @@ async function run(): Promise { } await asyncExec('pnpm clean:test-applications', { env, cwd: __dirname }); + await asyncExec('pnpm cache delete "@sentry/*"', { env, cwd: __dirname }); const testAppPaths = appName ? [appName.trim()] : globSync('*', { cwd: `${__dirname}/test-applications/` }); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts index 242b4c778a0e..a9f89152d56d 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.service.ts @@ -85,10 +85,21 @@ export class AppService { only supports minute granularity, but we don't want to wait (worst case) a full minute for the tests to finish. */ - @Cron('*/5 * * * * *', { name: 'test-cron-error' }) - @SentryCron('test-cron-error-slug', monitorConfig) + @Cron('*/5 * * * * *', { name: 'test-async-cron-error' }) + @SentryCron('test-async-cron-error-slug', monitorConfig) async testCronError() { - throw new Error('Test error from cron job'); + throw new Error('Test error from cron async job'); + } + + /* + Actual cron schedule differs from schedule defined in config because Sentry + only supports minute granularity, but we don't want to wait (worst case) a + full minute for the tests to finish. + */ + @Cron('*/5 * * * * *', { name: 'test-sync-cron-error' }) + @SentryCron('test-sync-cron-error-slug', monitorConfig) + testSyncCronError() { + throw new Error('Test error from cron sync job'); } async killTestCron(job: string) { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/cron-decorator.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/cron-decorator.test.ts index 7896603b3bd9..c193a94911c1 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/cron-decorator.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/cron-decorator.test.ts @@ -62,20 +62,36 @@ test('Cron job triggers send of in_progress envelope', async ({ baseURL }) => { await fetch(`${baseURL}/kill-test-cron/test-cron-job`); }); -test('Sends exceptions to Sentry on error in cron job', async ({ baseURL }) => { +test('Sends exceptions to Sentry on error in async cron job', async ({ baseURL }) => { const errorEventPromise = waitForError('nestjs-basic', event => { - return !event.type && event.exception?.values?.[0]?.value === 'Test error from cron job'; + return !event.type && event.exception?.values?.[0]?.value === 'Test error from cron async job'; }); const errorEvent = await errorEventPromise; expect(errorEvent.exception?.values).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.value).toBe('Test error from cron job'); expect(errorEvent.contexts?.trace).toEqual({ trace_id: expect.stringMatching(/[a-f0-9]{32}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), }); // kill cron so tests don't get stuck - await fetch(`${baseURL}/kill-test-cron/test-cron-error`); + await fetch(`${baseURL}/kill-test-cron/test-async-cron-error`); +}); + +test('Sends exceptions to Sentry on error in sync cron job', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs-basic', event => { + return !event.type && event.exception?.values?.[0]?.value === 'Test error from cron sync job'; + }); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + }); + + // kill cron so tests don't get stuck + await fetch(`${baseURL}/kill-test-cron/test-sync-cron-error`); }); diff --git a/dev-packages/e2e-tests/test-applications/node-express/src/app.ts b/dev-packages/e2e-tests/test-applications/node-express/src/app.ts index 76a02ea2c255..35b21a97b9aa 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/src/app.ts @@ -29,6 +29,18 @@ const port = 3030; app.use(mcpRouter); +app.get('/crash-in-with-monitor/:id', async (req, res) => { + try { + await Sentry.withMonitor('express-crash', async () => { + throw new Error(`This is an exception withMonitor: ${req.params.id}`); + }); + res.sendStatus(200); + } catch (error: any) { + res.status(500); + res.send({ message: error.message, pid: process.pid }); + } +}); + app.get('/test-success', function (req, res) { res.send({ version: 'v1' }); }); diff --git a/dev-packages/e2e-tests/test-applications/node-express/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-express/tests/errors.test.ts index bf0c5c5fb6b2..a4faaf137eb7 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/tests/errors.test.ts @@ -37,6 +37,17 @@ test('Should record caught exceptions with local variable', async ({ baseURL }) const errorEvent = await errorEventPromise; - const frames = errorEvent.exception?.values?.[0].stacktrace?.frames; - expect(frames?.[frames.length - 1].vars?.randomVariableToRecord).toBeDefined(); + const frames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + expect(frames?.[frames.length - 1]?.vars?.randomVariableToRecord).toBeDefined(); +}); + +test('To not crash app from withMonitor', async ({ baseURL }) => { + const doRequest = async (id: number) => { + const response = await fetch(`${baseURL}/crash-in-with-monitor/${id}`) + return response.json(); + } + const [response1, response2] = await Promise.all([doRequest(1), doRequest(2)]) + expect(response1.message).toBe('This is an exception withMonitor: 1') + expect(response2.message).toBe('This is an exception withMonitor: 2') + expect(response1.pid).toBe(response2.pid) //Just to double-check, TBS }); diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 92e4d09d4d81..5d36ffedf43f 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -150,6 +150,7 @@ export function captureCheckIn(checkIn: CheckIn, upsertMonitorConfig?: MonitorCo * Wraps a callback with a cron monitor check in. The check in will be sent to Sentry when the callback finishes. * * @param monitorSlug The distinct slug of the monitor. + * @param callback Callback to be monitored * @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want * to create a monitor automatically when sending a check in. */ @@ -175,18 +176,18 @@ export function withMonitor( } if (isThenable(maybePromiseResult)) { - Promise.resolve(maybePromiseResult).then( - () => { + return maybePromiseResult.then( + r => { finishCheckIn('ok'); + return r; }, e => { finishCheckIn('error'); throw e; }, - ); - } else { - finishCheckIn('ok'); + ) as T; } + finishCheckIn('ok'); return maybePromiseResult; }); diff --git a/packages/nestjs/src/decorators.ts b/packages/nestjs/src/decorators.ts index b11b69046c8c..9ac7315dabd8 100644 --- a/packages/nestjs/src/decorators.ts +++ b/packages/nestjs/src/decorators.ts @@ -1,5 +1,5 @@ import type { MonitorConfig } from '@sentry/core'; -import { captureException } from '@sentry/core'; +import { captureException, isThenable } from '@sentry/core'; import * as Sentry from '@sentry/node'; import { startSpan } from '@sentry/node'; import { isExpectedError } from './helpers'; @@ -15,7 +15,20 @@ export const SentryCron = (monitorSlug: string, monitorConfig?: MonitorConfig): return Sentry.withMonitor( monitorSlug, () => { - return originalMethod.apply(this, args); + let result; + try { + result = originalMethod.apply(this, args); + } catch (e) { + captureException(e); + throw e; + } + if (isThenable(result)) { + return result.then(undefined, e => { + captureException(e); + throw e; + }); + } + return result; }, monitorConfig, );