diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/test.ts b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/test.ts index 5a35991bfd4b..10981a84d103 100644 --- a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/test.ts @@ -1,6 +1,7 @@ import * as childProcess from 'child_process'; import * as path from 'path'; import { describe, expect, test } from 'vitest'; +import { conditionalTest } from '../../../utils'; import { createRunner } from '../../../utils/runner'; describe('OnUncaughtException integration', () => { @@ -101,4 +102,129 @@ describe('OnUncaughtException integration', () => { .start() .completed(); }); + + conditionalTest({ max: 18 })('Worker thread error handling Node 18', () => { + test('should capture uncaught worker thread errors - without childProcess integration', async () => { + await createRunner(__dirname, 'worker-thread/uncaught-worker.mjs') + .withInstrument(path.join(__dirname, 'worker-thread/instrument.mjs')) + .expect({ + event: { + level: 'fatal', + exception: { + values: [ + { + type: 'Error', + value: 'job failed', + mechanism: { + type: 'auto.node.onuncaughtexception', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }, + ], + }, + }, + }) + .start() + .completed(); + }); + }); + + // childProcessIntegration only exists in Node 20+ + conditionalTest({ min: 20 })('Worker thread error handling Node 20+', () => { + test.each(['mjs', 'js'])('should not interfere with worker thread error handling ".%s"', async extension => { + const runner = createRunner(__dirname, `worker-thread/caught-worker.${extension}`) + .withFlags( + extension === 'mjs' ? '--import' : '--require', + path.join(__dirname, `worker-thread/instrument.${extension}`), + ) + .expect({ + event: { + level: 'error', + exception: { + values: [ + { + type: 'Error', + value: 'job failed', + mechanism: { + type: 'auto.child_process.worker_thread', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }, + ], + }, + }, + }) + .start(); + + await runner.completed(); + + const logs = runner.getLogs(); + + expect(logs).toEqual(expect.arrayContaining([expect.stringMatching(/^caught Error: job failed/)])); + }); + + test('should not interfere with worker thread error handling when required inline', async () => { + const runner = createRunner(__dirname, 'worker-thread/caught-worker-inline.js') + .expect({ + event: { + level: 'error', + exception: { + values: [ + { + type: 'Error', + value: 'job failed', + mechanism: { + type: 'auto.child_process.worker_thread', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }, + ], + }, + }, + }) + .start(); + + await runner.completed(); + + const logs = runner.getLogs(); + + expect(logs).toEqual(expect.arrayContaining([expect.stringMatching(/^caught Error: job failed/)])); + }); + + test('should capture uncaught worker thread errors', async () => { + await createRunner(__dirname, 'worker-thread/uncaught-worker.mjs') + .withInstrument(path.join(__dirname, 'worker-thread/instrument.mjs')) + .expect({ + event: { + level: 'error', + exception: { + values: [ + { + type: 'Error', + value: 'job failed', + mechanism: { + type: 'auto.child_process.worker_thread', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }, + ], + }, + }, + }) + .start() + .completed(); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker-inline.js b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker-inline.js new file mode 100644 index 000000000000..798d6725308b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker-inline.js @@ -0,0 +1,4 @@ +// reuse the same worker script as the other tests +// just now in one file +require('./instrument.js'); +require('./caught-worker.js'); diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker.js b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker.js new file mode 100644 index 000000000000..a13112e06d92 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker.js @@ -0,0 +1,23 @@ +const path = require('path'); +const { Worker } = require('worker_threads'); + +function runJob() { + const worker = new Worker(path.join(__dirname, 'job.js')); + return new Promise((resolve, reject) => { + worker.once('error', reject); + worker.once('exit', code => { + if (code) reject(new Error(`Worker exited with code ${code}`)); + else resolve(); + }); + }); +} + +runJob() + .then(() => { + // eslint-disable-next-line no-console + console.log('Job completed successfully'); + }) + .catch(err => { + // eslint-disable-next-line no-console + console.error('caught', err); + }); diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker.mjs b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker.mjs new file mode 100644 index 000000000000..4e1750e36e71 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/caught-worker.mjs @@ -0,0 +1,24 @@ +import path from 'path'; +import { Worker } from 'worker_threads'; + +const __dirname = new URL('.', import.meta.url).pathname; + +function runJob() { + const worker = new Worker(path.join(__dirname, 'job.js')); + return new Promise((resolve, reject) => { + worker.once('error', reject); + worker.once('exit', code => { + if (code) reject(new Error(`Worker exited with code ${code}`)); + else resolve(); + }); + }); +} + +try { + await runJob(); + // eslint-disable-next-line no-console + console.log('Job completed successfully'); +} catch (err) { + // eslint-disable-next-line no-console + console.error('caught', err); +} diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/instrument.js b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/instrument.js new file mode 100644 index 000000000000..a2b13b91bce6 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/instrument.js @@ -0,0 +1,9 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 0, + debug: false, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/instrument.mjs b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/instrument.mjs new file mode 100644 index 000000000000..9263fe27bce1 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 0, + debug: false, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/job.js b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/job.js new file mode 100644 index 000000000000..b904a77813ac --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/job.js @@ -0,0 +1 @@ +throw new Error('job failed'); diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/uncaught-worker.mjs b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/uncaught-worker.mjs new file mode 100644 index 000000000000..eceff3cffa77 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/worker-thread/uncaught-worker.mjs @@ -0,0 +1,17 @@ +import path from 'path'; +import { Worker } from 'worker_threads'; + +const __dirname = new URL('.', import.meta.url).pathname; + +function runJob() { + const worker = new Worker(path.join(__dirname, 'job.js')); + return new Promise((resolve, reject) => { + worker.once('error', reject); + worker.once('exit', code => { + if (code) reject(new Error(`Worker exited with code ${code}`)); + else resolve(); + }); + }); +} + +await runJob(); diff --git a/packages/node-core/src/integrations/onuncaughtexception.ts b/packages/node-core/src/integrations/onuncaughtexception.ts index 41c4bf96917d..8afa70787a5c 100644 --- a/packages/node-core/src/integrations/onuncaughtexception.ts +++ b/packages/node-core/src/integrations/onuncaughtexception.ts @@ -1,4 +1,5 @@ import { captureException, debug, defineIntegration, getClient } from '@sentry/core'; +import { isMainThread } from 'worker_threads'; import { DEBUG_BUILD } from '../debug-build'; import type { NodeClient } from '../sdk/client'; import { logAndExitProcess } from '../utils/errorhandling'; @@ -44,6 +45,12 @@ export const onUncaughtExceptionIntegration = defineIntegration((options: Partia return { name: INTEGRATION_NAME, setup(client: NodeClient) { + // errors in worker threads are already handled by the childProcessIntegration + // also we don't want to exit the Node process on worker thread errors + if (!isMainThread) { + return; + } + global.process.on('uncaughtException', makeErrorHandler(client, optionsWithDefaults)); }, };