diff --git a/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/broken-stdio-pipe-test-script.js b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/broken-stdio-pipe-test-script.js new file mode 100644 index 000000000000..96c7c394a9a3 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/broken-stdio-pipe-test-script.js @@ -0,0 +1,12 @@ +const Sentry = require('@sentry/node'); + +// Unreachable rather than invalid, so `client.close()` is still pending while the +// broken pipe keeps erroring. +Sentry.init({ + traceLifecycle: 'static', + dsn: 'https://public@127.0.0.1:1/1337', +}); + +// The test runner closes both stdio streams, so this write raises EPIPE. Node ignores +// SIGPIPE, so it arrives as an uncaught exception. +setInterval(() => process.stdout.write('x'.repeat(4096)), 0); 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 c2f8aba9f225..55e22ba46558 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 @@ -47,6 +47,26 @@ describe('OnUncaughtException integration', () => { }); })); + test('should exit rather than recurse when stderr is a broken pipe', async () => { + const testScriptPath = path.resolve(__dirname, 'broken-stdio-pipe-test-script.js'); + + // The heap cap makes a regression fail in ~1s; at the default size it takes a minute + // and just looks like a hang. + const child = childProcess.spawn(process.execPath, ['--max-old-space-size=64', testScriptPath], { + stdio: ['ignore', 'pipe', 'pipe'], + }); + + child.stdout.destroy(); + child.stderr.destroy(); + + const exited = await new Promise<{ code: number | null; signal: string | null }>(resolve => { + child.on('exit', (code, signal) => resolve({ code, signal })); + }); + + // Unbounded recursion shows up as SIGABRT from the V8 OOM abort. + expect(exited).toEqual({ code: 1, signal: null }); + }); + describe('with `exitEvenIfOtherHandlersAreRegistered` set to false', () => { test('should close process on uncaught error with no additional listeners registered', () => new Promise(done => { diff --git a/packages/node/src/utils/errorhandling.ts b/packages/node/src/utils/errorhandling.ts index bb22766b5155..b293a569ec6a 100644 --- a/packages/node/src/utils/errorhandling.ts +++ b/packages/node/src/utils/errorhandling.ts @@ -4,10 +4,19 @@ import type { NodeClient } from '../sdk/client'; const DEFAULT_SHUTDOWN_TIMEOUT = 2000; +let isShuttingDown = false; + /** * @hidden */ export function logAndExitProcess(error: unknown): void { + // A broken stderr makes the console write below raise EPIPE, re-entering here. Return + // rather than exit, so the in-flight `client.close()` still flushes the fatal event. + if (isShuttingDown) { + return; + } + isShuttingDown = true; + consoleSandbox(() => { // eslint-disable-next-line no-console console.error(error); @@ -33,6 +42,7 @@ export function logAndExitProcess(error: unknown): void { }, error => { DEBUG_BUILD && debug.error(error); + global.process.exit(1); }, ); }