Skip to content

Commit

Permalink
fix(node): Fix cron instrumentation and add tests (#11811)
Browse files Browse the repository at this point in the history
Closes #11766 

These tests are also in TypeScript so they check the types too.

I found that two out of three cron libraries were actually swallowing
exceptions so that they were not captured by Sentry. I added calls to
`captureException` to rectify that.
  • Loading branch information
timfish committed May 7, 2024
1 parent 31d462e commit 55d41f7
Show file tree
Hide file tree
Showing 13 changed files with 444 additions and 40 deletions.
4 changes: 4 additions & 0 deletions dev-packages/node-integration-tests/package.json
Expand Up @@ -50,6 +50,8 @@
"mongoose": "^5.13.22",
"mysql": "^2.18.1",
"mysql2": "^3.7.1",
"node-cron": "^3.0.3",
"node-schedule": "^2.1.1",
"nock": "^13.1.0",
"pg": "^8.7.3",
"proxy": "^2.1.1",
Expand All @@ -58,6 +60,8 @@
"yargs": "^16.2.0"
},
"devDependencies": {
"@types/node-cron": "^3.0.11",
"@types/node-schedule": "^2.1.7",
"globby": "11"
},
"config": {
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/node-integration-tests/src/index.ts
Expand Up @@ -13,7 +13,7 @@ export function loggingTransport(_options: BaseTransportOptions): Transport {
return Promise.resolve({ statusCode: 200 });
},
flush(): PromiseLike<boolean> {
return Promise.resolve(true);
return new Promise(resolve => setTimeout(() => resolve(true), 1000));
},
};
}
Expand Down
31 changes: 31 additions & 0 deletions dev-packages/node-integration-tests/suites/cron/cron/scenario.ts
@@ -0,0 +1,31 @@
import { loggingTransport } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node';
import { CronJob } from 'cron';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
autoSessionTracking: false,
transport: loggingTransport,
});

const CronJobWithCheckIn = Sentry.cron.instrumentCron(CronJob, 'my-cron-job');

let closeNext = false;

const cron = new CronJobWithCheckIn('* * * * * *', () => {
if (closeNext) {
cron.stop();
throw new Error('Error in cron job');
}

// eslint-disable-next-line no-console
console.log('You will see this message every second');
closeNext = true;
});

cron.start();

setTimeout(() => {
process.exit();
}, 5000);
75 changes: 75 additions & 0 deletions dev-packages/node-integration-tests/suites/cron/cron/test.ts
@@ -0,0 +1,75 @@
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';

afterAll(() => {
cleanupChildProcesses();
});

test('cron instrumentation', done => {
createRunner(__dirname, 'scenario.ts')
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'in_progress',
release: '1.0',
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'ok',
release: '1.0',
duration: expect.any(Number),
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'in_progress',
release: '1.0',
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'error',
release: '1.0',
duration: expect.any(Number),
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
event: {
exception: { values: [{ type: 'Error', value: 'Error in cron job' }] },
},
})
.start(done);
});
@@ -1,9 +1,37 @@
import { loggingTransport } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node';
import { CronJob } from 'cron';
import * as cron from 'node-cron';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const CronJobWithCheckIn = Sentry.cron.instrumentCron(CronJob, 'my-cron-job');
Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
autoSessionTracking: false,
transport: loggingTransport,
});

const cronWithCheckIn = Sentry.cron.instrumentNodeCron(cron);

let closeNext = false;

const task = cronWithCheckIn.schedule(
'* * * * * *',
() => {
if (closeNext) {
// https://github.com/node-cron/node-cron/issues/317
setImmediate(() => {
task.stop();
});

throw new Error('Error in cron job');
}

// eslint-disable-next-line no-console
console.log('You will see this message every second');
closeNext = true;
},
{ name: 'my-cron-job' },
);

setTimeout(() => {
process.exit(0);
}, 1_000);
process.exit();
}, 5000);
70 changes: 68 additions & 2 deletions dev-packages/node-integration-tests/suites/cron/node-cron/test.ts
Expand Up @@ -4,6 +4,72 @@ afterAll(() => {
cleanupChildProcesses();
});

test('node-cron types should match', done => {
createRunner(__dirname, 'scenario.ts').ensureNoErrorOutput().start(done);
test('node-cron instrumentation', done => {
createRunner(__dirname, 'scenario.ts')
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'in_progress',
release: '1.0',
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'ok',
release: '1.0',
duration: expect.any(Number),
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'in_progress',
release: '1.0',
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'error',
release: '1.0',
duration: expect.any(Number),
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
event: {
exception: { values: [{ type: 'Error', value: 'Error in cron job' }] },
},
})
.start(done);
});
@@ -0,0 +1,29 @@
import { loggingTransport } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node';
import * as schedule from 'node-schedule';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
autoSessionTracking: false,
transport: loggingTransport,
});

const scheduleWithCheckIn = Sentry.cron.instrumentNodeSchedule(schedule);

let closeNext = false;

const job = scheduleWithCheckIn.scheduleJob('my-cron-job', '* * * * * *', () => {
if (closeNext) {
job.cancel();
throw new Error('Error in cron job');
}

// eslint-disable-next-line no-console
console.log('You will see this message every second');
closeNext = true;
});

setTimeout(() => {
process.exit();
}, 5000);
@@ -0,0 +1,75 @@
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';

afterAll(() => {
cleanupChildProcesses();
});

test('node-schedule instrumentation', done => {
createRunner(__dirname, 'scenario.ts')
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'in_progress',
release: '1.0',
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'ok',
release: '1.0',
duration: expect.any(Number),
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'in_progress',
release: '1.0',
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
check_in: {
check_in_id: expect.any(String),
monitor_slug: 'my-cron-job',
status: 'error',
release: '1.0',
duration: expect.any(Number),
contexts: {
trace: {
trace_id: expect.any(String),
span_id: expect.any(String),
},
},
},
})
.expect({
event: {
exception: { values: [{ type: 'Error', value: 'Error in cron job' }] },
},
})
.start(done);
});

0 comments on commit 55d41f7

Please sign in to comment.