diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts index 9ffc328a2c30..33584dea5e78 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/src/app.controller.ts @@ -86,6 +86,12 @@ export class AppController { return { result: await this.appService.testSpanDecoratorSync() }; } + @Get('test-derived-cron') + async testDerivedCron() { + await this.appService.testDerivedCron(); + return {}; + } + @Get('kill-test-cron/:job') async killTestCron(@Param('job') job: string) { this.appService.killTestCron(job); 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 a7b91b7b3d98..e9bb0c7f69c6 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 @@ -113,6 +113,13 @@ export class AppService { throw new Error('Test error from cron sync job'); } + // Disabled so it only runs through the `test-derived-cron` endpoint, without waiting for the schedule. + @Cron('0 30 9 * * 1-5', { name: 'test-derived-cron', timeZone: 'Europe/Vienna', disabled: true }) + @SentryCron('test-derived-cron-slug', { checkinMargin: 2 }) + async testDerivedCron() { + console.log('Test derived cron!'); + } + async killTestCron(job: string) { this.schedulerRegistry.deleteCronJob(job); } 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 6aeeae723a64..e1d006ccdb5d 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,6 +62,35 @@ test('Cron job triggers send of in_progress envelope', async ({ baseURL }) => { await fetch(`${baseURL}/kill-test-cron/test-cron-job`); }); +test('Sends the schedule and time zone of @Cron with the check-in', async ({ baseURL }) => { + const inProgressEnvelopePromise = waitForEnvelopeItem('nestjs-basic', envelope => { + return ( + envelope[0].type === 'check_in' && + envelope[1]['monitor_slug'] === 'test-derived-cron-slug' && + envelope[1]['status'] === 'in_progress' + ); + }); + + await fetch(`${baseURL}/test-derived-cron`); + + const inProgressEnvelope = await inProgressEnvelopePromise; + + expect(inProgressEnvelope[1]).toEqual( + expect.objectContaining({ + monitor_slug: 'test-derived-cron-slug', + status: 'in_progress', + monitor_config: { + schedule: { + type: 'crontab', + value: '30 9 * * 1-5', + }, + timezone: 'Europe/Vienna', + checkin_margin: 2, + }, + }), + ); +}); + test('Sends exceptions to Sentry on error in async cron job', async ({ baseURL }) => { const errorEventPromise = waitForError('nestjs-basic', event => { return ( diff --git a/packages/nestjs/src/decorators.ts b/packages/nestjs/src/decorators.ts index e4690c988d7a..f50b5edf1545 100644 --- a/packages/nestjs/src/decorators.ts +++ b/packages/nestjs/src/decorators.ts @@ -1,34 +1,213 @@ import type { MonitorConfig } from '@sentry/core'; import { CODE_FUNCTION_NAME } from '@sentry/conventions/attributes'; -import { captureException, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; +import { captureException, debug, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import * as Sentry from '@sentry/node'; import { startSpan } from '@sentry/node'; +import { DEBUG_BUILD } from './debug-build'; import { isExpectedError } from './helpers'; +import type { ReflectWithMetadata } from './integrations/helpers'; import { copyReflectMetadata } from './integrations/helpers'; +/** + * Monitor settings for `@SentryCron` whose schedule and time zone come from the `@Cron()` decorator + * of `@nestjs/schedule` on the same method. Set `fromCronDecorator: false` to not send them. + */ +export type SentryCronMonitorSettings = Omit & { + schedule?: never; + timezone?: never; + fromCronDecorator?: boolean; +}; + /** * A decorator wrapping the native nest Cron decorator, sending check-ins to Sentry. + * + * Unless a monitor config with a `schedule` is passed, the schedule and time zone of the method's + * `@Cron()` decorator are sent with each check-in, so Sentry can create the monitor on the first run. */ -export const SentryCron = (monitorSlug: string, monitorConfig?: MonitorConfig): MethodDecorator => { +export const SentryCron = ( + monitorSlug: string, + monitorConfig?: (MonitorConfig & { fromCronDecorator?: never }) | SentryCronMonitorSettings, +): MethodDecorator => { return (target: unknown, propertyKey, descriptor: PropertyDescriptor) => { const originalMethod = descriptor.value as (...args: unknown[]) => Promise; - descriptor.value = function (...args: unknown[]) { + let resolvedMonitorConfig: MonitorConfig | undefined; + let resolved = false; + + const wrappedMethod = function (this: unknown, ...args: unknown[]): unknown { + if (!resolved) { + resolved = true; + // `@Cron()` sets its metadata on whatever function is `descriptor.value` when it runs, which is + // this function if it is applied after `@SentryCron()`, so it is only readable at call time. + resolvedMonitorConfig = resolveMonitorConfig(monitorSlug, monitorConfig, [ + wrappedMethod, + (target as Record | undefined)?.[propertyKey], + ]); + } + return Sentry.withMonitor( monitorSlug, () => { return originalMethod.apply(this, args); }, - monitorConfig, + resolvedMonitorConfig, ); }; + descriptor.value = wrappedMethod; + copyFunctionNameAndMetadata({ originalMethod, descriptor }); return descriptor; }; }; +function resolveMonitorConfig( + monitorSlug: string, + monitorConfig: MonitorConfig | SentryCronMonitorSettings | undefined, + candidates: unknown[], +): MonitorConfig | undefined { + if (monitorConfig?.schedule) { + return monitorConfig; + } + + const { fromCronDecorator = true, ...monitorSettings } = monitorConfig || {}; + const cronConfig = fromCronDecorator ? getMonitorConfigFromNestCron(candidates) : undefined; + + if (!cronConfig) { + if (DEBUG_BUILD && Object.keys(monitorSettings).length) { + const reason = fromCronDecorator + ? 'no schedule could be taken from @Cron()' + : 'fromCronDecorator is false and no schedule was passed'; + debug.warn( + `[SentryCron] The monitor settings for "${monitorSlug}" are not sent, because ${reason}. A monitor config needs a schedule.`, + ); + } + return undefined; + } + + return { ...monitorSettings, ...cronConfig }; +} + +const SCHEDULE_CRON_OPTIONS = 'SCHEDULE_CRON_OPTIONS'; + +// Presets of the `cron` package (which also lowercases them), as sent to Sentry. Sentry accepts +// `@yearly`/`@annually`/`@monthly`/`@weekly`/`@daily`/`@hourly`; the others are sent as crontabs. +const CRON_PRESETS: Record = { + '@yearly': '@yearly', + '@annually': '@annually', + '@monthly': '@monthly', + '@weekly': '@weekly', + '@daily': '@daily', + '@hourly': '@hourly', + '@midnight': '0 0 * * *', + '@minutely': '* * * * *', + '@weekdays': '0 0 * * 1-5', + '@weekends': '0 0 * * 0,6', +}; + +interface NestCronOptions { + cronTime?: unknown; + timeZone?: unknown; + utcOffset?: unknown; +} + +function getMonitorConfigFromNestCron(candidates: unknown[]): MonitorConfig | undefined { + const R = Reflect as ReflectWithMetadata; + if (typeof R.getMetadata !== 'function') { + return undefined; + } + + for (const candidate of candidates) { + if (typeof candidate !== 'function') { + continue; + } + const cronOptions = R.getMetadata(SCHEDULE_CRON_OPTIONS, candidate); + if (cronOptions && typeof cronOptions === 'object') { + return nestCronOptionsToMonitorConfig(cronOptions); + } + } + + return undefined; +} + +/** + * Converts the options `@Cron()` stores as metadata into a Sentry monitor config. + * Returns `undefined` for schedules Sentry can't represent. + */ +function nestCronOptionsToMonitorConfig(cronOptions: NestCronOptions): MonitorConfig | undefined { + const { cronTime, timeZone, utcOffset } = cronOptions; + + // A fixed UTC offset has no IANA time zone equivalent. + if (typeof cronTime !== 'string' || utcOffset != null) { + return undefined; + } + + const fields = cronTime.trim().split(/\s+/); + let crontab: string | undefined; + if (fields.length === 1) { + crontab = CRON_PRESETS[(fields[0] as string).toLowerCase()]; + } else if (fields.length === 5) { + crontab = isSupportedCrontab(fields) ? fields.join(' ') : undefined; + } else if (fields.length === 6 && /^\d+$/.test(fields[0] as string)) { + // Sentry schedules have minute granularity, so only a fixed second can be dropped. + crontab = isSupportedCrontab(fields.slice(1)) ? fields.slice(1).join(' ') : undefined; + } + + if (!crontab) { + return undefined; + } + + // Without a `timeZone`, the job runs in the server's local time zone. + const timezone = typeof timeZone === 'string' && timeZone ? timeZone : getLocalTimeZone(); + + // Without a time zone Sentry would assume UTC, which may not be when the job runs. + if (!timezone || !isSentryTimeZone(timezone)) { + return undefined; + } + + return { + schedule: { type: 'crontab', value: crontab }, + timezone, + }; +} + +/** + * Whether Sentry reads the 5 crontab fields the same way `cron` (used by `@nestjs/schedule`) runs them. + */ +function isSupportedCrontab([, , dayOfMonth, month, dayOfWeek]: string[]): boolean { + // `cron` 2.x (`@nestjs/schedule` 3) counts months from 0, so a numeric month is ambiguous. + if (/\d/.test(month as string)) { + return false; + } + + // With both day fields set, `cron` runs on either, but Sentry needs both when one starts with `*` (like `*/2`). + return !(dayOfMonth !== '*' && dayOfWeek !== '*' && (dayOfMonth?.startsWith('*') || dayOfWeek?.startsWith('*'))); +} + +/** + * Whether Sentry accepts the time zone: an IANA name, not a fixed offset like `UTC+3`. + */ +function isSentryTimeZone(timezone: string): boolean { + if (timezone === 'Etc/Unknown' || /^(?:utc|gmt)?[+-]/i.test(timezone)) { + return false; + } + try { + new Intl.DateTimeFormat('en-US', { timeZone: timezone }); + return true; + } catch { + return false; + } +} + +function getLocalTimeZone(): string | undefined { + try { + return Intl.DateTimeFormat().resolvedOptions().timeZone || undefined; + } catch { + return undefined; + } +} + /** * A decorator usable to wrap arbitrary functions with spans. */ diff --git a/packages/nestjs/src/index.ts b/packages/nestjs/src/index.ts index b96fb4c5390a..10a3400ee400 100644 --- a/packages/nestjs/src/index.ts +++ b/packages/nestjs/src/index.ts @@ -8,3 +8,4 @@ export { nestIntegration } from './integrations/nest'; export { getDefaultIntegrations, init } from './sdk'; export { SentryCron, SentryExceptionCaptured, SentryTraced } from './decorators'; +export type { SentryCronMonitorSettings } from './decorators'; diff --git a/packages/nestjs/test/decorators.test.ts b/packages/nestjs/test/decorators.test.ts index 9640244de437..80bd76ba57e0 100644 --- a/packages/nestjs/test/decorators.test.ts +++ b/packages/nestjs/test/decorators.test.ts @@ -1,8 +1,9 @@ import 'reflect-metadata'; +import { SetMetadata } from '@nestjs/common'; import { CODE_FUNCTION_NAME } from '@sentry/conventions/attributes'; import * as core from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SentryCron, SentryExceptionCaptured, SentryTraced } from '../src/decorators'; import * as helpers from '../src/helpers'; @@ -259,6 +260,216 @@ describe('SentryCron decorator', () => { }); }); +describe('SentryCron decorator with @Cron', () => { + // Mirrors `@Cron()` of `@nestjs/schedule`, which stores its options under this metadata key. + const Cron = (cronTime: unknown, options: Record = {}): MethodDecorator => + SetMetadata('SCHEDULE_CRON_OPTIONS', { ...options, cronTime }); + + // Mirrors the SDK's own `@Cron()` instrumentation, which swaps the method before nest stores the metadata. + const WrappingCron = + (cronTime: unknown): MethodDecorator => + (target, propertyKey, descriptor) => { + const original = descriptor.value as unknown as (...args: unknown[]) => unknown; + (descriptor as PropertyDescriptor).value = function (this: unknown, ...args: unknown[]) { + return original.apply(this, args); + }; + return Cron(cronTime)(target, propertyKey, descriptor); + }; + + // Applies decorators the way TypeScript does: listed top to bottom, applied bottom to top. + function decorate(...decorators: MethodDecorator[]): { job: () => Promise } { + class Service {} + let descriptor: PropertyDescriptor = { + value: async () => 'done', + writable: true, + enumerable: false, + configurable: true, + }; + for (const decorator of [...decorators].reverse()) { + descriptor = (decorator(Service.prototype, 'job', descriptor) as PropertyDescriptor | undefined) ?? descriptor; + } + Object.defineProperty(Service.prototype, 'job', descriptor); + return new Service() as { job: () => Promise }; + } + + beforeEach(() => { + // `@Cron()` without a `timeZone` runs in the server's local time zone. + vi.spyOn(Intl.DateTimeFormat.prototype, 'resolvedOptions').mockReturnValue({ + timeZone: 'Asia/Tokyo', + } as Intl.ResolvedDateTimeFormatOptions); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('derives the monitor config when @SentryCron is above @Cron', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job'), Cron('0 * * * *')); + + expect(await service.job()).toBe('done'); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value: '0 * * * *' }, + timezone: 'Asia/Tokyo', + }); + }); + + it('derives the monitor config when @Cron is above @SentryCron', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(Cron('0 * * * *'), SentryCron('my-job')); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value: '0 * * * *' }, + timezone: 'Asia/Tokyo', + }); + }); + + it('derives the monitor config when @Cron replaces the method', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(WrappingCron('0 * * * *'), SentryCron('my-job')); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value: '0 * * * *' }, + timezone: 'Asia/Tokyo', + }); + }); + + it('drops a fixed seconds field and passes the time zone', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job'), Cron('0 30 9 * * 1-5', { timeZone: 'Europe/Vienna' })); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value: '30 9 * * 1-5' }, + timezone: 'Europe/Vienna', + }); + }); + + it.each([ + ['a sub-minute schedule', Cron('*/5 * * * * *')], + ['a one-off date', Cron(new Date())], + ['an unknown preset', Cron('@reboot')], + ['a utc offset', Cron('0 * * * *', { utcOffset: 120 })], + ['a numeric month', Cron('0 9 1 5 *')], + ['a day-of-month step with a day of week', Cron('0 9 */2 * MON')], + ['a fixed-offset time zone', Cron('0 * * * *', { timeZone: 'UTC+3' })], + ['an unknown time zone', Cron('0 * * * *', { timeZone: 'Mars/Olympus' })], + ])('sends no monitor config for %s', async (_, cronDecorator) => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job'), cronDecorator); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), undefined); + }); + + it.each([ + ['a month name', '0 9 1 MAY *'], + ['both day fields', '0 9 1-7 * MON'], + ])('sends a crontab with %s', async (_, crontab) => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job'), Cron(crontab)); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value: crontab }, + timezone: 'Asia/Tokyo', + }); + }); + + it('sends no monitor config when the local time zone is unknown', async () => { + vi.spyOn(Intl.DateTimeFormat.prototype, 'resolvedOptions').mockReturnValue({ + timeZone: 'Etc/Unknown', + } as Intl.ResolvedDateTimeFormatOptions); + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job'), Cron('0 * * * *')); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), undefined); + }); + + it.each([ + ['@hourly', '@hourly'], + ['@daily', '@daily'], + ['@WEEKLY', '@weekly'], + ['@monthly', '@monthly'], + ['@yearly', '@yearly'], + ['@annually', '@annually'], + ['@midnight', '0 0 * * *'], + ['@weekdays', '0 0 * * 1-5'], + ])('sends the preset %s as %s', async (preset, value) => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job'), Cron(preset)); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value }, + timezone: 'Asia/Tokyo', + }); + }); + + it('warns when monitor settings are passed but no schedule can be derived', async () => { + const warnSpy = vi.spyOn(core.debug, 'warn').mockImplementation(() => undefined); + const withMonitorSpy = vi.spyOn(core, 'withMonitor').mockImplementation((_, callback) => callback()); + const service = decorate(SentryCron('my-job', { checkinMargin: 2 }), Cron('*/5 * * * * *')); + + await service.job(); + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), undefined); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('"my-job"')); + }); + + it('does not warn when no monitor settings are passed', async () => { + const warnSpy = vi.spyOn(core.debug, 'warn').mockImplementation(() => undefined); + vi.spyOn(core, 'withMonitor').mockImplementation((_, callback) => callback()); + const service = decorate(SentryCron('my-job'), Cron('*/5 * * * * *')); + + await service.job(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('sends no monitor config with fromCronDecorator: false', async () => { + const warnSpy = vi.spyOn(core.debug, 'warn').mockImplementation(() => undefined); + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job', { fromCronDecorator: false, checkinMargin: 2 }), Cron('0 * * * *')); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), undefined); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('fromCronDecorator is false')); + }); + + it('keeps the other monitor settings', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const service = decorate(SentryCron('my-job', { checkinMargin: 2, maxRuntime: 10 }), Cron('0 * * * *')); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), { + schedule: { type: 'crontab', value: '0 * * * *' }, + timezone: 'Asia/Tokyo', + checkinMargin: 2, + maxRuntime: 10, + }); + }); + + it('uses an explicit monitor config as is', async () => { + const withMonitorSpy = vi.spyOn(core, 'withMonitor'); + const monitorConfig: core.MonitorConfig = { schedule: { type: 'interval', value: 1, unit: 'hour' } }; + const service = decorate(SentryCron('my-job', monitorConfig), Cron('0 * * * *')); + + await service.job(); + expect(withMonitorSpy).toHaveBeenCalledWith('my-job', expect.any(Function), monitorConfig); + }); + + it('does not accept a schedule or time zone together with fromCronDecorator', () => { + // @ts-expect-error - `fromCronDecorator` only applies to settings without a schedule + SentryCron('my-job', { schedule: { type: 'crontab', value: '0 * * * *' }, fromCronDecorator: false }); + // @ts-expect-error - the time zone comes from `@Cron()` + SentryCron('my-job', { timezone: 'Europe/Vienna', checkinMargin: 2 }); + }); +}); + describe('SentryExceptionCaptured decorator', () => { beforeEach(() => { vi.clearAllMocks();