From 36441bbb56413d3f2e32f977d5933e179b2d9ed4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 21 Mar 2023 18:59:08 +0000 Subject: [PATCH 1/3] Consider the tracing error for process exit --- packages/core/src/tracing/errors.ts | 4 ++++ packages/node/src/integrations/onuncaughtexception.ts | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/tracing/errors.ts b/packages/core/src/tracing/errors.ts index 3351f428fecf..1105143e69f8 100644 --- a/packages/core/src/tracing/errors.ts +++ b/packages/core/src/tracing/errors.ts @@ -29,3 +29,7 @@ function errorCallback(): void { activeTransaction.setStatus(status); } } + +// The function name will be lost when bundling but we need to be able to identify this listener later to maintain the +// node.js default exit behaviour +errorCallback.name = 'tracingErrorCallback'; diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index 2d10ae61d696..3e22db192a4a 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -99,8 +99,10 @@ export class OnUncaughtException implements Integration { .listeners('uncaughtException') .reduce((acc, listener) => { if ( + // There are 3 listeners we ignore: listener.name === 'domainUncaughtExceptionClear' || // as soon as we're using domains this listener is attached by node itself - listener === this.handler // filter the handler we registered ourselves) + listener.name === 'tracingErrorCallback' || // the handler we register for tracing + listener === this.handler // the handler we register in this integration ) { return acc; } else { From f488211a39b0619ecfad6b52881701fa67d632e1 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 21 Mar 2023 19:33:00 +0000 Subject: [PATCH 2/3] rename to avoid collisions --- packages/core/src/tracing/errors.ts | 2 +- packages/node/src/integrations/onuncaughtexception.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tracing/errors.ts b/packages/core/src/tracing/errors.ts index 1105143e69f8..0628b2e3ddd6 100644 --- a/packages/core/src/tracing/errors.ts +++ b/packages/core/src/tracing/errors.ts @@ -32,4 +32,4 @@ function errorCallback(): void { // The function name will be lost when bundling but we need to be able to identify this listener later to maintain the // node.js default exit behaviour -errorCallback.name = 'tracingErrorCallback'; +errorCallback.name = 'sentry_tracingErrorCallback'; diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index 3e22db192a4a..916a00e40b46 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -101,7 +101,7 @@ export class OnUncaughtException implements Integration { if ( // There are 3 listeners we ignore: listener.name === 'domainUncaughtExceptionClear' || // as soon as we're using domains this listener is attached by node itself - listener.name === 'tracingErrorCallback' || // the handler we register for tracing + listener.name === 'sentry_tracingErrorCallback' || // the handler we register for tracing listener === this.handler // the handler we register in this integration ) { return acc; From ca0fc4dd28fff565094dab1b606286aa27ccafc0 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 21 Mar 2023 19:57:03 +0000 Subject: [PATCH 3/3] Don't try and override function name --- packages/core/src/tracing/errors.ts | 2 +- .../src/integrations/onuncaughtexception.ts | 33 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/core/src/tracing/errors.ts b/packages/core/src/tracing/errors.ts index 0628b2e3ddd6..8785bd86d448 100644 --- a/packages/core/src/tracing/errors.ts +++ b/packages/core/src/tracing/errors.ts @@ -32,4 +32,4 @@ function errorCallback(): void { // The function name will be lost when bundling but we need to be able to identify this listener later to maintain the // node.js default exit behaviour -errorCallback.name = 'sentry_tracingErrorCallback'; +errorCallback.tag = 'sentry_tracingErrorCallback'; diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index 916a00e40b46..e55de4d1fd8e 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -8,6 +8,10 @@ import { logAndExitProcess } from './utils/errorhandling'; type OnFatalErrorHandler = (firstError: Error, secondError?: Error) => void; +type TaggedListener = NodeJS.UncaughtExceptionListener & { + tag?: string; +}; + // CAREFUL: Please think twice before updating the way _options looks because the Next.js SDK depends on it in `index.server.ts` interface OnUncaughtExceptionOptions { // TODO(v8): Evaluate whether we should switch the default behaviour here. @@ -95,20 +99,21 @@ export class OnUncaughtException implements Integration { // exit behaviour of the SDK accordingly: // - If other listeners are attached, do not exit. // - If the only listener attached is ours, exit. - const userProvidedListenersCount = global.process - .listeners('uncaughtException') - .reduce((acc, listener) => { - if ( - // There are 3 listeners we ignore: - listener.name === 'domainUncaughtExceptionClear' || // as soon as we're using domains this listener is attached by node itself - listener.name === 'sentry_tracingErrorCallback' || // the handler we register for tracing - listener === this.handler // the handler we register in this integration - ) { - return acc; - } else { - return acc + 1; - } - }, 0); + const userProvidedListenersCount = ( + global.process.listeners('uncaughtException') as TaggedListener[] + ).reduce((acc, listener) => { + if ( + // There are 3 listeners we ignore: + listener.name === 'domainUncaughtExceptionClear' || // as soon as we're using domains this listener is attached by node itself + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + (listener.tag && listener.tag === 'sentry_tracingErrorCallback') || // the handler we register for tracing + listener === this.handler // the handler we register in this integration + ) { + return acc; + } else { + return acc + 1; + } + }, 0); const processWouldExit = userProvidedListenersCount === 0; const shouldApplyFatalHandlingLogic = this._options.exitEvenIfOtherHandlersAreRegistered || processWouldExit;