From 07b4c11720529a385d222fc24707e1700e5eba94 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Tue, 26 May 2026 14:21:44 +0200 Subject: [PATCH 1/5] ref(node): Stop using `registerSpanErrorInstrumentation()` on server --- packages/core/src/server-runtime-client.ts | 4 ---- packages/core/src/tracing/errors.ts | 4 ---- packages/node-core/src/integrations/onuncaughtexception.ts | 2 -- 3 files changed, 10 deletions(-) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index d969f3580114..26ae3e80f3b1 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -3,7 +3,6 @@ import { Client } from './client'; import { getIsolationScope } from './currentScopes'; import { DEBUG_BUILD } from './debug-build'; import type { Scope } from './scope'; -import { registerSpanErrorInstrumentation } from './tracing'; import { DEFAULT_TRANSPORT_BUFFER_SIZE } from './transports/base'; import { addUserAgentToTransportHeaders } from './transports/userAgent'; import type { CheckIn, MonitorConfig, SerializedCheckIn } from './types/checkin'; @@ -38,9 +37,6 @@ export class ServerRuntimeClient< * @param options Configuration options for this SDK. */ public constructor(options: O) { - // Server clients always support tracing - registerSpanErrorInstrumentation(); - addUserAgentToTransportHeaders(options); super(options); diff --git a/packages/core/src/tracing/errors.ts b/packages/core/src/tracing/errors.ts index cf7aafbac8ea..bccc4225f61b 100644 --- a/packages/core/src/tracing/errors.ts +++ b/packages/core/src/tracing/errors.ts @@ -33,10 +33,6 @@ export function registerSpanErrorInstrumentation(): 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.tag = 'sentry_tracingErrorCallback'; - errorsInstrumented = true; addGlobalErrorInstrumentationHandler(errorCallback); addGlobalUnhandledRejectionInstrumentationHandler(errorCallback); diff --git a/packages/node-core/src/integrations/onuncaughtexception.ts b/packages/node-core/src/integrations/onuncaughtexception.ts index 8afa70787a5c..5300b31df54c 100644 --- a/packages/node-core/src/integrations/onuncaughtexception.ts +++ b/packages/node-core/src/integrations/onuncaughtexception.ts @@ -89,8 +89,6 @@ export function makeErrorHandler(client: NodeClient, options: OnUncaughtExceptio return ( // as soon as we're using domains this listener is attached by node itself listener.name !== 'domainUncaughtExceptionClear' && - // the handler we register for tracing - listener.tag !== 'sentry_tracingErrorCallback' && // the handler we register in this integration (listener as ErrorHandler)._errorHandler !== true ); From a65e856f73fbecfc09cd7239a3febfa97647fa13 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Wed, 27 May 2026 09:45:17 +0200 Subject: [PATCH 2/5] add node-integration-test --- .../suites/express/handle-error/test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/express/handle-error/test.ts b/dev-packages/node-integration-tests/suites/express/handle-error/test.ts index 4e702360242f..e66850393c78 100644 --- a/dev-packages/node-integration-tests/suites/express/handle-error/test.ts +++ b/dev-packages/node-integration-tests/suites/express/handle-error/test.ts @@ -10,6 +10,15 @@ test('should capture and send Express controller error with txn name if tracesSa .expect({ transaction: { transaction: 'GET /test/express/:id', + contexts: { + trace: { + op: 'http.server', + status: 'internal_error', + data: expect.objectContaining({ + 'http.response.status_code': 500, + }), + }, + }, }, }) .expect({ From d1be7c0c9c0836639b5c827e2b0b59e1e2269de5 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Wed, 27 May 2026 11:05:10 +0200 Subject: [PATCH 3/5] add otel test --- .../instrument.mjs | 8 +++++ .../scenario.mjs | 15 +++++++++ .../tracer-start-active-span-error/test.ts | 33 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/instrument.mjs new file mode 100644 index 000000000000..62e64bf8dc10 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/instrument.mjs @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/scenario.mjs new file mode 100644 index 000000000000..3e1e31f2871e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/scenario.mjs @@ -0,0 +1,15 @@ +import * as Sentry from '@sentry/node'; + +const tracer = Sentry.getClient().tracer; + +async function run() { + await tracer.startActiveSpan('test span name', async span => { + try { + throw new Error('Test error from tracer.startActiveSpan'); + } finally { + span.end(); + } + }); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts new file mode 100644 index 000000000000..f8dff9a85c60 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts @@ -0,0 +1,33 @@ +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; + +describe('tracer.startActiveSpan errors', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + // When a callback to raw OTel `tracer.startActiveSpan` throws and the error propagates + // (uncaught), the span status is NOT automatically marked as errored. The user's `finally` + // calls `span.end()` before the error propagates, and an OTel span becomes immutable on end. + // + // Users who want auto-status-on-error should use `Sentry.startSpan` instead, or follow the + // OTel-idiomatic pattern: `span.recordException(err); span.setStatus({ code: ERROR })` in a + // `catch` inside the callback. + test('does NOT mark span errored when uncaught error escapes raw tracer.startActiveSpan callback (status quo)', async () => { + await createRunner() + .expect({ + transaction: { + transaction: 'test span name', + contexts: { + trace: { + status: 'ok', + }, + }, + }, + }) + .start() + .completed(); + }); + }); +}); From 6589a838859ca98fed910ade8420f0659a2e6e41 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Wed, 27 May 2026 11:11:37 +0200 Subject: [PATCH 4/5] fix test name --- .../suites/tracing/tracer-start-active-span-error/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts index f8dff9a85c60..8d79aa952252 100644 --- a/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/tracer-start-active-span-error/test.ts @@ -14,7 +14,7 @@ describe('tracer.startActiveSpan errors', () => { // Users who want auto-status-on-error should use `Sentry.startSpan` instead, or follow the // OTel-idiomatic pattern: `span.recordException(err); span.setStatus({ code: ERROR })` in a // `catch` inside the callback. - test('does NOT mark span errored when uncaught error escapes raw tracer.startActiveSpan callback (status quo)', async () => { + test('does NOT mark span errored when uncaught error escapes raw tracer.startActiveSpan callback', async () => { await createRunner() .expect({ transaction: { From 0adec401b2a134bf429c13fb30a2c8b877ee3a43 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Wed, 27 May 2026 11:41:36 +0200 Subject: [PATCH 5/5] remove tagged thing --- .../src/integrations/onuncaughtexception.ts | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/node-core/src/integrations/onuncaughtexception.ts b/packages/node-core/src/integrations/onuncaughtexception.ts index 5300b31df54c..ea0cd877e8be 100644 --- a/packages/node-core/src/integrations/onuncaughtexception.ts +++ b/packages/node-core/src/integrations/onuncaughtexception.ts @@ -6,10 +6,6 @@ import { logAndExitProcess } from '../utils/errorhandling'; type OnFatalErrorHandler = (firstError: Error, secondError?: Error) => void; -type TaggedListener = NodeJS.UncaughtExceptionListener & { - tag?: string; -}; - interface OnUncaughtExceptionOptions { /** * Controls if the SDK should register a handler to exit the process on uncaught errors: @@ -83,17 +79,15 @@ export function makeErrorHandler(client: NodeClient, options: OnUncaughtExceptio // 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') as TaggedListener[]).filter( - listener => { - // There are 3 listeners we ignore: - return ( - // as soon as we're using domains this listener is attached by node itself - listener.name !== 'domainUncaughtExceptionClear' && - // the handler we register in this integration - (listener as ErrorHandler)._errorHandler !== true - ); - }, - ).length; + const userProvidedListenersCount = global.process.listeners('uncaughtException').filter(listener => { + // There are 3 listeners we ignore: + return ( + // as soon as we're using domains this listener is attached by node itself + listener.name !== 'domainUncaughtExceptionClear' && + // the handler we register in this integration + (listener as ErrorHandler)._errorHandler !== true + ); + }).length; const processWouldExit = userProvidedListenersCount === 0; const shouldApplyFatalHandlingLogic = options.exitEvenIfOtherHandlersAreRegistered || processWouldExit;