diff --git a/MIGRATION.md b/MIGRATION.md index 51bf19cddb01..046906b2f0fd 100644 --- a/MIGRATION.md +++ b/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 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 c90dd989e37f..60523dc37684 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 @@ -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(); }); }); diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index de5ac52c4f18..d961d6717e4c 100644 --- a/packages/nextjs/src/server/index.ts +++ b/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(), ]; diff --git a/packages/nextjs/src/server/onUncaughtExceptionIntegration.ts b/packages/nextjs/src/server/onUncaughtExceptionIntegration.ts deleted file mode 100644 index d9eada65fa95..000000000000 --- a/packages/nextjs/src/server/onUncaughtExceptionIntegration.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { onUncaughtExceptionIntegration as originalOnUncaughtExceptionIntegration } from '@sentry/node'; - -export const onUncaughtExceptionIntegration: typeof originalOnUncaughtExceptionIntegration = options => { - return originalOnUncaughtExceptionIntegration({ ...options, exitEvenIfOtherHandlersAreRegistered: false }); -}; diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index e56c3c0801d7..637174f12351 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.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 = {}) => { - const _options = { - exitEvenIfOtherHandlersAreRegistered: true, +/** + * Add a global exception handler. + */ +export const onUncaughtExceptionIntegration = defineIntegration((options: Partial = {}) => { + 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);