feat(node): Do not exit process by default when other onUncaughtException handlers are registered in onUncaughtExceptionIntegration (getsentry#11532)

Author: Luca Forstner

MIGRATION.md

@@ -1118,6 +1118,7 @@ Sentry.init({
 - [Updated behaviour of `extraErrorDataIntegration`](./MIGRATION.md#extraerrordataintegration-changes)
 - [Updated behaviour of `transactionContext` passed to `tracesSampler`](./MIGRATION.md#transactioncontext-no-longer-passed-to-tracessampler)
 - [Updated behaviour of `getClient()`](./MIGRATION.md#getclient-always-returns-a-client)
+- [Updated behaviour of the SDK in combination with `onUncaughtException` handlers in Node.js](./MIGRATION.md#behaviour-in-combination-with-onuncaughtexception-handlers-in-node.js)
 - [Removal of Client-Side health check transaction filters](./MIGRATION.md#removal-of-client-side-health-check-transaction-filters)
 - [Change of Replay default options (`unblock` and `unmask`)](./MIGRATION.md#change-of-replay-default-options-unblock-and-unmask)
 - [Angular Tracing Decorator renaming](./MIGRATION.md#angular-tracing-decorator-renaming)
@@ -1168,6 +1169,16 @@ some attributes may only be set later during the span lifecycle (and thus not be
 Sentry was actually initialized, using `getClient()` will thus not work anymore. Instead, you should use the new
 `Sentry.isInitialized()` utility to check this.
 
+#### Behaviour in combination with `onUncaughtException` handlers in Node.js
+
+Previously the SDK exited the process by default, even though additional `onUncaughtException` may have been registered,
+that would have prevented the process from exiting. You could opt out of this behaviour by setting the
+`exitEvenIfOtherHandlersAreRegistered: false` in the `onUncaughtExceptionIntegration` options. Up until now the value
+for this option defaulted to `true`.
+
+Going forward, the default value for `exitEvenIfOtherHandlersAreRegistered` will be `false`, meaning that the SDK will
+not exit your process when you have registered other `onUncaughtException` handlers.
+
 #### Removal of Client-Side health check transaction filters
 
 The SDK no longer filters out health check transactions by default. Instead, they are sent to Sentry but still dropped

dev-packages/node-integration-tests/suites/public-api/OnUncaughtException/test.ts

@@ -15,15 +15,14 @@ describe('OnUncaughtException integration', () => {
     });
   });
 
-  test('should close process on uncaught error when additional listeners are registered', done => {
-    expect.assertions(3);
+  test('should not close process on uncaught error when additional listeners are registered', done => {
+    expect.assertions(2);
 
     const testScriptPath = path.resolve(__dirname, 'additional-listener-test-script.js');
 
     childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (err, stdout) => {
-      expect(err).not.toBeNull();
-      expect(err?.code).toBe(1);
-      expect(stdout).not.toBe("I'm alive!");
+      expect(err).toBeNull();
+      expect(stdout).toBe("I'm alive!");
       done();
     });
   });

packages/nextjs/src/server/index.ts

@@ -8,14 +8,12 @@ import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolica
 import { getVercelEnv } from '../common/getVercelEnv';
 import { isBuild } from '../common/utils/isBuild';
 import { distDirRewriteFramesIntegration } from './distDirRewriteFramesIntegration';
-import { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration';
 
 export * from '@sentry/node';
 import type { EventProcessor } from '@sentry/types';
 import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration';
 
 export { captureUnderscoreErrorException } from '../common/_error';
-export { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration';
 
 const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
   __rewriteFramesDistDir__?: string;
@@ -75,13 +73,11 @@ export function init(options: NodeOptions): void {
   const customDefaultIntegrations = [
     ...getDefaultIntegrations(options).filter(
       integration =>
-        integration.name !== 'OnUncaughtException' &&
         // Next.js comes with its own Node-Fetch instrumentation, so we shouldn't add ours on-top
         integration.name !== 'NodeFetch' &&
         // Next.js comes with its own Http instrumentation for OTel which lead to double spans for route handler requests
         integration.name !== 'Http',
     ),
-    onUncaughtExceptionIntegration(),
     requestIsolationScopeIntegration(),
   ];
 

packages/nextjs/src/server/onUncaughtExceptionIntegration.ts

@@ -1,6 +1,5 @@
 import { captureException, defineIntegration } from '@sentry/core';
 import { getClient } from '@sentry/core';
-import type { IntegrationFn } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
@@ -13,17 +12,13 @@ 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.
-  // Also, we can evaluate using https://nodejs.org/api/process.html#event-uncaughtexceptionmonitor per default, and
-  // falling back to current behaviour when that's not available.
   /**
    * Controls if the SDK should register a handler to exit the process on uncaught errors:
    * - `true`: The SDK will exit the process on all uncaught errors.
    * - `false`: The SDK will only exit the process when there are no other `uncaughtException` handlers attached.
    *
-   * Default: `true`
+   * Default: `false`
    */
   exitEvenIfOtherHandlersAreRegistered: boolean;
 
@@ -40,24 +35,22 @@ interface OnUncaughtExceptionOptions {
 
 const INTEGRATION_NAME = 'OnUncaughtException';
 
-const _onUncaughtExceptionIntegration = ((options: Partial<OnUncaughtExceptionOptions> = {}) => {
-  const _options = {
-    exitEvenIfOtherHandlersAreRegistered: true,
+/**
+ * Add a global exception handler.
+ */
+export const onUncaughtExceptionIntegration = defineIntegration((options: Partial<OnUncaughtExceptionOptions> = {}) => {
+  const optionsWithDefaults = {
+    exitEvenIfOtherHandlersAreRegistered: false,
     ...options,
   };
 
   return {
     name: INTEGRATION_NAME,
     setup(client: NodeClient) {
-      global.process.on('uncaughtException', makeErrorHandler(client, _options));
+      global.process.on('uncaughtException', makeErrorHandler(client, optionsWithDefaults));
     },
   };
-}) satisfies IntegrationFn;
-
-/**
- * Add a global exception handler.
- */
-export const onUncaughtExceptionIntegration = defineIntegration(_onUncaughtExceptionIntegration);
+});
 
 type ErrorHandler = { _errorHandler: boolean } & ((error: Error) => void);