Skip to content

fix(node): Consider tracing error handler for process exit #7558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/core/src/tracing/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.tag = 'sentry_tracingErrorCallback';
31 changes: 19 additions & 12 deletions packages/node/src/integrations/onuncaughtexception.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -95,18 +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<number>((acc, listener) => {
if (
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)
) {
return acc;
} else {
return acc + 1;
}
}, 0);
const userProvidedListenersCount = (
global.process.listeners('uncaughtException') as TaggedListener[]
).reduce<number>((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;
Expand Down