Skip to content

Commit 55d41f7

Browse files
timfishtimfish
authored
fix(node): Fix cron instrumentation and add tests (#11811)
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.
1 parent 31d462e commit 55d41f7

File tree

13 files changed

+444
-40
lines changed

13 files changed

+444
-40
lines changed

‎dev-packages/node-integration-tests/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
"mongoose": "^5.13.22",
5151
"mysql": "^2.18.1",
5252
"mysql2": "^3.7.1",
53+
"node-cron": "^3.0.3",
54+
"node-schedule": "^2.1.1",
5355
"nock": "^13.1.0",
5456
"pg": "^8.7.3",
5557
"proxy": "^2.1.1",
@@ -58,6 +60,8 @@
5860
"yargs": "^16.2.0"
5961
},
6062
"devDependencies": {
63+
"@types/node-cron": "^3.0.11",
64+
"@types/node-schedule": "^2.1.7",
6165
"globby": "11"
6266
},
6367
"config": {

‎dev-packages/node-integration-tests/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export function loggingTransport(_options: BaseTransportOptions): Transport {
1313
return Promise.resolve({ statusCode: 200 });
1414
},
1515
flush(): PromiseLike<boolean> {
16-
return Promise.resolve(true);
16+
return new Promise(resolve => setTimeout(() => resolve(true), 1000));
1717
},
1818
};
1919
}

‎dev-packages/node-integration-tests/suites/cron/cron/scenario.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
import { CronJob } from 'cron';
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
release: '1.0',
8+
autoSessionTracking: false,
9+
transport: loggingTransport,
10+
});
11+
12+
const CronJobWithCheckIn = Sentry.cron.instrumentCron(CronJob, 'my-cron-job');
13+
14+
let closeNext = false;
15+
16+
const cron = new CronJobWithCheckIn('* * * * * *', () => {
17+
if (closeNext) {
18+
cron.stop();
19+
throw new Error('Error in cron job');
20+
}
21+
22+
// eslint-disable-next-line no-console
23+
console.log('You will see this message every second');
24+
closeNext = true;
25+
});
26+
27+
cron.start();
28+
29+
setTimeout(() => {
30+
process.exit();
31+
}, 5000);

‎dev-packages/node-integration-tests/suites/cron/cron/test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
2+
3+
afterAll(() => {
4+
cleanupChildProcesses();
5+
});
6+
7+
test('cron instrumentation', done => {
8+
createRunner(__dirname, 'scenario.ts')
9+
.expect({
10+
check_in: {
11+
check_in_id: expect.any(String),
12+
monitor_slug: 'my-cron-job',
13+
status: 'in_progress',
14+
release: '1.0',
15+
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
16+
contexts: {
17+
trace: {
18+
trace_id: expect.any(String),
19+
span_id: expect.any(String),
20+
},
21+
},
22+
},
23+
})
24+
.expect({
25+
check_in: {
26+
check_in_id: expect.any(String),
27+
monitor_slug: 'my-cron-job',
28+
status: 'ok',
29+
release: '1.0',
30+
duration: expect.any(Number),
31+
contexts: {
32+
trace: {
33+
trace_id: expect.any(String),
34+
span_id: expect.any(String),
35+
},
36+
},
37+
},
38+
})
39+
.expect({
40+
check_in: {
41+
check_in_id: expect.any(String),
42+
monitor_slug: 'my-cron-job',
43+
status: 'in_progress',
44+
release: '1.0',
45+
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
46+
contexts: {
47+
trace: {
48+
trace_id: expect.any(String),
49+
span_id: expect.any(String),
50+
},
51+
},
52+
},
53+
})
54+
.expect({
55+
check_in: {
56+
check_in_id: expect.any(String),
57+
monitor_slug: 'my-cron-job',
58+
status: 'error',
59+
release: '1.0',
60+
duration: expect.any(Number),
61+
contexts: {
62+
trace: {
63+
trace_id: expect.any(String),
64+
span_id: expect.any(String),
65+
},
66+
},
67+
},
68+
})
69+
.expect({
70+
event: {
71+
exception: { values: [{ type: 'Error', value: 'Error in cron job' }] },
72+
},
73+
})
74+
.start(done);
75+
});

‎dev-packages/node-integration-tests/suites/cron/node-cron/scenario.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,37 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
12
import * as Sentry from '@sentry/node';
2-
import { CronJob } from 'cron';
3+
import * as cron from 'node-cron';
34

4-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
5-
const CronJobWithCheckIn = Sentry.cron.instrumentCron(CronJob, 'my-cron-job');
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
release: '1.0',
8+
autoSessionTracking: false,
9+
transport: loggingTransport,
10+
});
11+
12+
const cronWithCheckIn = Sentry.cron.instrumentNodeCron(cron);
13+
14+
let closeNext = false;
15+
16+
const task = cronWithCheckIn.schedule(
17+
'* * * * * *',
18+
() => {
19+
if (closeNext) {
20+
// https://github.com/node-cron/node-cron/issues/317
21+
setImmediate(() => {
22+
task.stop();
23+
});
24+
25+
throw new Error('Error in cron job');
26+
}
27+
28+
// eslint-disable-next-line no-console
29+
console.log('You will see this message every second');
30+
closeNext = true;
31+
},
32+
{ name: 'my-cron-job' },
33+
);
634

735
setTimeout(() => {
8-
process.exit(0);
9-
}, 1_000);
36+
process.exit();
37+
}, 5000);

‎dev-packages/node-integration-tests/suites/cron/node-cron/test.ts

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,72 @@ afterAll(() => {
44
cleanupChildProcesses();
55
});
66

7-
test('node-cron types should match', done => {
8-
createRunner(__dirname, 'scenario.ts').ensureNoErrorOutput().start(done);
7+
test('node-cron instrumentation', done => {
8+
createRunner(__dirname, 'scenario.ts')
9+
.expect({
10+
check_in: {
11+
check_in_id: expect.any(String),
12+
monitor_slug: 'my-cron-job',
13+
status: 'in_progress',
14+
release: '1.0',
15+
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
16+
contexts: {
17+
trace: {
18+
trace_id: expect.any(String),
19+
span_id: expect.any(String),
20+
},
21+
},
22+
},
23+
})
24+
.expect({
25+
check_in: {
26+
check_in_id: expect.any(String),
27+
monitor_slug: 'my-cron-job',
28+
status: 'ok',
29+
release: '1.0',
30+
duration: expect.any(Number),
31+
contexts: {
32+
trace: {
33+
trace_id: expect.any(String),
34+
span_id: expect.any(String),
35+
},
36+
},
37+
},
38+
})
39+
.expect({
40+
check_in: {
41+
check_in_id: expect.any(String),
42+
monitor_slug: 'my-cron-job',
43+
status: 'in_progress',
44+
release: '1.0',
45+
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
46+
contexts: {
47+
trace: {
48+
trace_id: expect.any(String),
49+
span_id: expect.any(String),
50+
},
51+
},
52+
},
53+
})
54+
.expect({
55+
check_in: {
56+
check_in_id: expect.any(String),
57+
monitor_slug: 'my-cron-job',
58+
status: 'error',
59+
release: '1.0',
60+
duration: expect.any(Number),
61+
contexts: {
62+
trace: {
63+
trace_id: expect.any(String),
64+
span_id: expect.any(String),
65+
},
66+
},
67+
},
68+
})
69+
.expect({
70+
event: {
71+
exception: { values: [{ type: 'Error', value: 'Error in cron job' }] },
72+
},
73+
})
74+
.start(done);
975
});

‎dev-packages/node-integration-tests/suites/cron/node-schedule/scenario.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
import * as schedule from 'node-schedule';
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
release: '1.0',
8+
autoSessionTracking: false,
9+
transport: loggingTransport,
10+
});
11+
12+
const scheduleWithCheckIn = Sentry.cron.instrumentNodeSchedule(schedule);
13+
14+
let closeNext = false;
15+
16+
const job = scheduleWithCheckIn.scheduleJob('my-cron-job', '* * * * * *', () => {
17+
if (closeNext) {
18+
job.cancel();
19+
throw new Error('Error in cron job');
20+
}
21+
22+
// eslint-disable-next-line no-console
23+
console.log('You will see this message every second');
24+
closeNext = true;
25+
});
26+
27+
setTimeout(() => {
28+
process.exit();
29+
}, 5000);

‎dev-packages/node-integration-tests/suites/cron/node-schedule/test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
2+
3+
afterAll(() => {
4+
cleanupChildProcesses();
5+
});
6+
7+
test('node-schedule instrumentation', done => {
8+
createRunner(__dirname, 'scenario.ts')
9+
.expect({
10+
check_in: {
11+
check_in_id: expect.any(String),
12+
monitor_slug: 'my-cron-job',
13+
status: 'in_progress',
14+
release: '1.0',
15+
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
16+
contexts: {
17+
trace: {
18+
trace_id: expect.any(String),
19+
span_id: expect.any(String),
20+
},
21+
},
22+
},
23+
})
24+
.expect({
25+
check_in: {
26+
check_in_id: expect.any(String),
27+
monitor_slug: 'my-cron-job',
28+
status: 'ok',
29+
release: '1.0',
30+
duration: expect.any(Number),
31+
contexts: {
32+
trace: {
33+
trace_id: expect.any(String),
34+
span_id: expect.any(String),
35+
},
36+
},
37+
},
38+
})
39+
.expect({
40+
check_in: {
41+
check_in_id: expect.any(String),
42+
monitor_slug: 'my-cron-job',
43+
status: 'in_progress',
44+
release: '1.0',
45+
monitor_config: { schedule: { type: 'crontab', value: '* * * * * *' } },
46+
contexts: {
47+
trace: {
48+
trace_id: expect.any(String),
49+
span_id: expect.any(String),
50+
},
51+
},
52+
},
53+
})
54+
.expect({
55+
check_in: {
56+
check_in_id: expect.any(String),
57+
monitor_slug: 'my-cron-job',
58+
status: 'error',
59+
release: '1.0',
60+
duration: expect.any(Number),
61+
contexts: {
62+
trace: {
63+
trace_id: expect.any(String),
64+
span_id: expect.any(String),
65+
},
66+
},
67+
},
68+
})
69+
.expect({
70+
event: {
71+
exception: { values: [{ type: 'Error', value: 'Error in cron job' }] },
72+
},
73+
})
74+
.start(done);
75+
});

0 commit comments

Comments
 (0)