From a5b70bcad3ec8dbbf7eaf0b2ef760b84610772e0 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 17 Jun 2026 09:47:31 +0200 Subject: [PATCH 1/5] feat(core): Add `bindScopeToEmitter` to bind a scope to an event emitter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new `bindScopeToEmitter(emitter, scope?)` API to core. It captures a scope (defaulting to the current scope) and patches the emitter's listener registration methods so that any listener added afterwards runs with that scope — and therefore the active span — active, even when it fires later in a different async context. This is useful when instrumenting APIs that hand back an event emitter (e.g. a streamed database query) whose `'data'`/`'error'`/`'end'` listeners would otherwise lose the trace context. It mirrors the event-emitter behavior of OpenTelemetry's `ContextManager.bind`, scoped to emitters only. Works with both Node.js `EventEmitter`s (`on`, `addListener`, ...) and DOM `EventTarget`s (`addEventListener`); the isolation scope is intentionally not captured as it is carried along by the active async context. Exported from all SDKs and covered by unit, node-integration and browser-integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../suites/tracing/bindScopeToEmitter/init.js | 8 + .../tracing/bindScopeToEmitter/subject.js | 21 ++ .../suites/tracing/bindScopeToEmitter/test.ts | 26 ++ .../public-api/bindScopeToEmitter/scenario.ts | 44 +++ .../public-api/bindScopeToEmitter/test.ts | 40 +++ packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + .../src/index.bundle.tracing.logs.metrics.ts | 1 + ...le.tracing.replay.feedback.logs.metrics.ts | 1 + .../index.bundle.tracing.replay.feedback.ts | 1 + ...ndex.bundle.tracing.replay.logs.metrics.ts | 1 + .../src/index.bundle.tracing.replay.ts | 1 + packages/browser/src/index.bundle.tracing.ts | 1 + packages/browser/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/cloudflare/src/index.ts | 1 + .../core/src/tracing/bindScopeToEmitter.ts | 164 ++++++++++ packages/core/src/tracing/index.ts | 1 + .../lib/tracing/bindScopeToEmitter.test.ts | 299 ++++++++++++++++++ packages/deno/src/index.ts | 1 + packages/elysia/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node-core/src/common-exports.ts | 1 + packages/node/src/index.ts | 1 + packages/remix/src/cloudflare/index.ts | 1 + packages/remix/src/server/index.ts | 1 + packages/solidstart/src/server/index.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + packages/sveltekit/src/worker/index.ts | 1 + packages/vercel-edge/src/index.ts | 1 + 30 files changed, 625 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts create mode 100644 dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/test.ts create mode 100644 packages/core/src/tracing/bindScopeToEmitter.ts create mode 100644 packages/core/test/lib/tracing/bindScopeToEmitter.test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/init.js b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/init.js new file mode 100644 index 000000000000..7c200c542c56 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/subject.js b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/subject.js new file mode 100644 index 000000000000..5c19d5ca9793 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/subject.js @@ -0,0 +1,21 @@ +// A browser-native event target. +const target = new EventTarget(); + +const parentSpan = Sentry.startInactiveSpan({ name: 'parent' }); + +// Bind + register the listener while `parentSpan` is the active span. +Sentry.withActiveSpan(parentSpan, () => { + Sentry.bindScopeToEmitter(target); + + target.addEventListener('data', () => { + Sentry.startSpan({ name: 'child-bound' }, () => { + // noop + }); + }); +}); + +// At this point no span is active. Dispatching should re-enter the bound (parent) scope, +// so `child-bound` is nested under `parent` rather than starting its own trace. +target.dispatchEvent(new Event('data')); + +parentSpan.end(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts new file mode 100644 index 000000000000..0f4a7bdffc0f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts @@ -0,0 +1,26 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers'; + +sentryTest('bindScopeToEmitter runs listeners with the bound scope active', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const req = waitForTransactionRequest(page, e => e.transaction === 'parent'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + const parentEvent = envelopeRequestParser(await req); + const parentSpanId = parentEvent.contexts?.trace?.span_id; + const parentTraceId = parentEvent.contexts?.trace?.trace_id; + expect(parentSpanId).toMatch(/[a-f\d]{16}/); + + // The listener fired while no span was active, yet `child-bound` is nested under `parent` + // because the parent scope was bound to the emitter. + const childBound = parentEvent.spans?.find(s => s.description === 'child-bound'); + expect(childBound).toBeDefined(); + expect(childBound?.parent_span_id).toBe(parentSpanId); + expect(childBound?.trace_id).toBe(parentTraceId); +}); diff --git a/dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/scenario.ts b/dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/scenario.ts new file mode 100644 index 000000000000..a4a256d3171d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/scenario.ts @@ -0,0 +1,44 @@ +import { EventEmitter } from 'node:events'; +import type { Span } from '@sentry/core'; +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [], + transport: loggingTransport, +}); + +const boundEmitter = new EventEmitter(); +const unboundEmitter = new EventEmitter(); + +let parentSpan: Span; + +Sentry.startSpanManual({ name: 'parent' }, span => { + parentSpan = span; + + // Bind the current (parent) scope to the emitter. Listeners registered afterwards should run + // with the parent span active, even when they fire in a different async context. + Sentry.bindScopeToEmitter(boundEmitter); + + boundEmitter.on('data', () => { + Sentry.startSpan({ name: 'child-bound' }, () => undefined); + }); + + // The unbound emitter is the control: its listener should NOT see the parent span. + unboundEmitter.on('data', () => { + Sentry.startSpan({ name: 'child-unbound' }, () => undefined); + }); +}); + +// Emit from a fresh async context (a timer scheduled at the top level), where the parent span is +// no longer active. Only the bound emitter should re-enter the parent scope. +setTimeout(() => { + unboundEmitter.emit('data'); + boundEmitter.emit('data'); + parentSpan.end(); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.flush(); +}, 10); diff --git a/dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/test.ts b/dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/test.ts new file mode 100644 index 000000000000..1ce679449100 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/bindScopeToEmitter/test.ts @@ -0,0 +1,40 @@ +import type { TransactionEvent } from '@sentry/core'; +import { afterAll, expect, test } from 'vitest'; +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('bindScopeToEmitter preserves the active span for listeners firing in a different async context', async () => { + // Collect both transactions regardless of the order they are flushed in. + const transactions: Record = {}; + const collect = (event: TransactionEvent): void => { + transactions[event.transaction as string] = event; + }; + + await createRunner(__dirname, 'scenario.ts') + .expect({ transaction: collect }) + .expect({ transaction: collect }) + .start() + .completed(); + + const parent = transactions['parent']; + const childUnbound = transactions['child-unbound']; + + expect(parent).toBeDefined(); + expect(childUnbound).toBeDefined(); + + const parentTraceId = parent?.contexts?.trace?.trace_id; + const parentSpanId = parent?.contexts?.trace?.span_id; + + // The bound emitter's listener ran inside the parent span context -> nested child span. + const childBound = parent?.spans?.find(span => span.description === 'child-bound'); + expect(childBound).toBeDefined(); + expect(childBound?.parent_span_id).toBe(parentSpanId); + expect(childBound?.trace_id).toBe(parentTraceId); + + // The unbound emitter's listener ran without the parent active -> its own, separate trace. + expect(childUnbound?.spans).toEqual([]); + expect(childUnbound?.contexts?.trace?.trace_id).not.toBe(parentTraceId); +}); diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index b4fc6fccbee3..f07e9c86c613 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -136,6 +136,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, startSession, startSpan, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 0cbc893b4601..844e2f1e2f03 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -79,6 +79,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getRootSpan, diff --git a/packages/browser/src/index.bundle.tracing.logs.metrics.ts b/packages/browser/src/index.bundle.tracing.logs.metrics.ts index 1dd34bb2ff42..7e9139e153aa 100644 --- a/packages/browser/src/index.bundle.tracing.logs.metrics.ts +++ b/packages/browser/src/index.bundle.tracing.logs.metrics.ts @@ -15,6 +15,7 @@ export { setMeasurement, startInactiveSpan, startNewTrace, + bindScopeToEmitter, startSpan, startSpanManual, withActiveSpan, diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts index cc7262dc9dbe..355877526bd4 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts @@ -13,6 +13,7 @@ export { setMeasurement, startInactiveSpan, startNewTrace, + bindScopeToEmitter, startSpan, startSpanManual, withActiveSpan, diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index 1e821b38f824..6147eca88328 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -20,6 +20,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, withActiveSpan, getSpanDescendants, setMeasurement, diff --git a/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts b/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts index 9abdd4675d5e..96def57ab8e0 100644 --- a/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts +++ b/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts @@ -15,6 +15,7 @@ export { setMeasurement, startInactiveSpan, startNewTrace, + bindScopeToEmitter, startSpan, startSpanManual, withActiveSpan, diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index 50da2dac87c4..990d1c3bbf4d 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -20,6 +20,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, withActiveSpan, getSpanDescendants, setMeasurement, diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index aac6825ecc65..db7de22b9c3e 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -21,6 +21,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, withActiveSpan, getSpanDescendants, setMeasurement, diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index ed35b8b4ac81..1abd577a7170 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -59,6 +59,7 @@ export { startSpanManual, withActiveSpan, startNewTrace, + bindScopeToEmitter, getSpanDescendants, setMeasurement, getSpanStatusFromHttpCode, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index c44742a0e2d3..9938465a06c7 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -101,6 +101,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getRootSpan, diff --git a/packages/cloudflare/src/index.ts b/packages/cloudflare/src/index.ts index 61707365fafd..852b48c33eab 100644 --- a/packages/cloudflare/src/index.ts +++ b/packages/cloudflare/src/index.ts @@ -66,6 +66,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getSpanDescendants, diff --git a/packages/core/src/tracing/bindScopeToEmitter.ts b/packages/core/src/tracing/bindScopeToEmitter.ts new file mode 100644 index 000000000000..a5b6d75f4a55 --- /dev/null +++ b/packages/core/src/tracing/bindScopeToEmitter.ts @@ -0,0 +1,164 @@ +import { getCurrentScope, withScope } from '../currentScopes'; +import type { Scope } from '../scope'; + +type BoundListener = (...args: unknown[]) => unknown; + +/** Per-event map from user-provided listeners to their scope-bound wrappers. */ +type ListenerPatchMap = Record | undefined>; + +// We patch both Node.js `EventEmitter` registration methods (`on`, `addListener`, ...) and the DOM +// `EventTarget.addEventListener`, so this works for Node emitters and browser-native event targets. + +/** Listener-registration methods we patch so listeners inherit the bound scope. */ +const ADD_LISTENER_METHODS = [ + 'addListener', + 'on', + 'once', + 'prependListener', + 'prependOnceListener', + 'addEventListener', +] as const; +/** Listener-removal methods we patch so removals find the scope-bound wrapper. */ +const REMOVE_LISTENER_METHODS = ['removeListener', 'off', 'removeEventListener'] as const; + +/** Symbol under which the patch map is stashed on a bound emitter. */ +const SCOPE_BOUND_LISTENERS = Symbol('SentryScopeBoundListeners'); + +/** + * Minimal structural type for a Node.js-style `EventEmitter` or DOM `EventTarget`. We intentionally + * avoid importing `node:events` so this stays usable in non-Node environments — objects without any + * of these methods simply pass through untouched. + */ +type EventEmitterLike = Record; + +// Guards against double-wrapping when a patched `on`/`addListener` delegates to another patched +// registration method internally. Binding is synchronous, so a module-level flag is safe here. +let isAddingBoundListener = false; + +/** + * Binds a scope to the given event emitter, so that any listener added to it runs with that scope + * (and therefore the active span) active — even if the listener fires later, in a different async + * context. + * + * By default the currently active scope is bound, captured at the time this function is called. + * Pass an explicit `scope` to bind a different one. + * + * This is useful when instrumenting APIs that hand back an event emitter (e.g. a streamed database + * query) whose `'data'` / `'error'` / `'end'` listeners would otherwise lose the trace context. + * + * Works with both Node.js `EventEmitter`s (`on`, `addListener`, ...) and DOM `EventTarget`s + * (`addEventListener`). Objects exposing none of these methods are returned untouched. + * + * The isolation scope is intentionally not captured — it is carried along by the active async + * context. This mirrors the event-emitter behavior of OpenTelemetry's `ContextManager.bind`. + */ +export function bindScopeToEmitter(emitter: T, scope: Scope = getCurrentScope()): T { + const ee = emitter as EventEmitterLike; + + // Already bound -> nothing to do. + if (getPatchMap(ee)) { + return emitter; + } + + createPatchMap(ee); + + for (const methodName of ADD_LISTENER_METHODS) { + if (typeof ee[methodName] !== 'function') { + continue; + } + ee[methodName] = patchAddListener(ee, ee[methodName] as BoundListener, scope); + } + + for (const methodName of REMOVE_LISTENER_METHODS) { + if (typeof ee[methodName] !== 'function') { + continue; + } + ee[methodName] = patchRemoveListener(ee, ee[methodName] as BoundListener); + } + + if (typeof ee.removeAllListeners === 'function') { + ee.removeAllListeners = patchRemoveAllListeners(ee, ee.removeAllListeners as BoundListener); + } + + return emitter; +} + +/** Wraps a listener so it runs with the given scope active. */ +function bindListenerToScope(listener: BoundListener, scope: Scope): BoundListener { + return function (this: unknown, ...args: unknown[]) { + return withScope(scope, () => listener.apply(this, args)); + }; +} + +function patchAddListener(ee: EventEmitterLike, original: BoundListener, scope: Scope): BoundListener { + return function (this: unknown, ...args: unknown[]) { + const event = args[0] as string; + const listener = args[1]; + // Extra args (e.g. the `options` argument of `addEventListener`) must be forwarded verbatim. + const rest = args.slice(2); + + // Pass through anything we can't wrap: re-entrant registrations and non-function listeners + // (e.g. `EventListener` objects passed to `addEventListener`). + if (isAddingBoundListener || typeof listener !== 'function') { + return original.apply(this, args); + } + + const map = getPatchMap(ee) || createPatchMap(ee); + let listeners = map[event]; + if (!listeners) { + listeners = new WeakMap(); + map[event] = listeners; + } + + const boundListener = bindListenerToScope(listener as BoundListener, scope); + listeners.set(listener as BoundListener, boundListener); + + isAddingBoundListener = true; + try { + return original.call(this, event, boundListener, ...rest); + } finally { + isAddingBoundListener = false; + } + }; +} + +function patchRemoveListener(ee: EventEmitterLike, original: BoundListener): BoundListener { + return function (this: unknown, ...args: unknown[]) { + const event = args[0] as string; + const listener = args[1]; + const rest = args.slice(2); + + const listeners = getPatchMap(ee)?.[event]; + if (!listeners || typeof listener !== 'function') { + return original.apply(this, args); + } + const boundListener = listeners.get(listener as BoundListener); + return original.call(this, event, boundListener || (listener as BoundListener), ...rest); + }; +} + +function patchRemoveAllListeners(ee: EventEmitterLike, original: BoundListener): BoundListener { + return function (this: unknown, ...args: unknown[]) { + const map = getPatchMap(ee); + if (map) { + if (args.length === 0) { + // `removeAllListeners()` with no event clears everything -> reset the map. + createPatchMap(ee); + } else { + const event = args[0] as string; + map[event] = undefined; + } + } + return original.apply(this, args); + }; +} + +function createPatchMap(ee: EventEmitterLike): ListenerPatchMap { + const map = Object.create(null) as ListenerPatchMap; + (ee as Record)[SCOPE_BOUND_LISTENERS] = map; + return map; +} + +function getPatchMap(ee: EventEmitterLike): ListenerPatchMap | undefined { + return (ee as Record)[SCOPE_BOUND_LISTENERS]; +} diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts index 9b56045b37f3..543d8242615a 100644 --- a/packages/core/src/tracing/index.ts +++ b/packages/core/src/tracing/index.ts @@ -15,6 +15,7 @@ export { startNewTrace, SUPPRESS_TRACING_KEY, } from './trace'; +export { bindScopeToEmitter } from './bindScopeToEmitter'; export { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan, diff --git a/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts b/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts new file mode 100644 index 000000000000..ba79e1570f4d --- /dev/null +++ b/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts @@ -0,0 +1,299 @@ +import { EventEmitter } from 'node:events'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getCurrentScope, getGlobalScope, getIsolationScope, setCurrentClient, withScope } from '../../../src'; +import { Scope } from '../../../src/scope'; +import { bindScopeToEmitter } from '../../../src/tracing/bindScopeToEmitter'; +import { startInactiveSpan, withActiveSpan } from '../../../src/tracing/trace'; +import { getActiveSpan } from '../../../src/utils/spanUtils'; +import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; + +describe('bindScopeToEmitter', () => { + beforeEach(() => { + getCurrentScope().clear(); + getIsolationScope().clear(); + getGlobalScope().clear(); + + const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); + setCurrentClient(client); + client.init(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('runs listeners added after binding with the scope active at bind time', () => { + const emitter = new EventEmitter(); + + let boundScope: Scope | undefined; + withScope(scope => { + boundScope = scope; + bindScopeToEmitter(emitter); + }); + + // The listener is registered *outside* the `withScope`, yet should see the bound scope. + let scopeInListener: Scope | undefined; + emitter.on('data', () => { + scopeInListener = getCurrentScope(); + }); + + emitter.emit('data'); + + expect(scopeInListener).toBe(boundScope); + expect(scopeInListener).not.toBe(getCurrentScope()); + }); + + it('binds an explicitly passed scope instead of the current one', () => { + const emitter = new EventEmitter(); + + const explicitScope = new Scope(); + bindScopeToEmitter(emitter, explicitScope); + + let scopeInListener: Scope | undefined; + emitter.on('data', () => { + scopeInListener = getCurrentScope(); + }); + + emitter.emit('data'); + + expect(scopeInListener).toBe(explicitScope); + expect(scopeInListener).not.toBe(getCurrentScope()); + }); + + it('prefers the explicitly passed scope over the active scope at call time', () => { + const emitter = new EventEmitter(); + + const explicitScope = new Scope(); + withScope(activeScope => { + // Bind a *different* scope than the one that is currently active. + expect(activeScope).not.toBe(explicitScope); + bindScopeToEmitter(emitter, explicitScope); + }); + + let scopeInListener: Scope | undefined; + emitter.on('data', () => { + scopeInListener = getCurrentScope(); + }); + + emitter.emit('data'); + + expect(scopeInListener).toBe(explicitScope); + }); + + it('preserves the active span for listeners', () => { + const emitter = new EventEmitter(); + + const span = startInactiveSpan({ name: 'parent' }); + withActiveSpan(span, () => { + bindScopeToEmitter(emitter); + }); + + let activeSpanInListener: ReturnType; + emitter.on('end', () => { + activeSpanInListener = getActiveSpan(); + }); + + expect(getActiveSpan()).toBeUndefined(); + emitter.emit('end'); + + expect(activeSpanInListener).toBe(span); + }); + + it.each(['on', 'addListener', 'prependListener'] as const)('binds the scope for listeners added via %s', method => { + const emitter = new EventEmitter(); + + let boundScope: Scope | undefined; + withScope(scope => { + boundScope = scope; + bindScopeToEmitter(emitter); + }); + + let scopeInListener: Scope | undefined; + emitter[method]('data', () => { + scopeInListener = getCurrentScope(); + }); + + emitter.emit('data'); + + expect(scopeInListener).toBe(boundScope); + }); + + it('binds the scope for `once` listeners and only fires them once', () => { + const emitter = new EventEmitter(); + + let boundScope: Scope | undefined; + withScope(scope => { + boundScope = scope; + bindScopeToEmitter(emitter); + }); + + const listener = vi.fn(() => getCurrentScope()); + emitter.once('data', listener); + + emitter.emit('data'); + emitter.emit('data'); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveReturnedWith(boundScope); + }); + + it('removes the wrapped listener when removing via the original reference', () => { + const emitter = new EventEmitter(); + bindScopeToEmitter(emitter); + + const listener = vi.fn(); + emitter.on('data', listener); + emitter.removeListener('data', listener); + + emitter.emit('data'); + + expect(listener).not.toHaveBeenCalled(); + expect(emitter.listenerCount('data')).toBe(0); + }); + + it('removes the wrapped listener via `off`', () => { + const emitter = new EventEmitter(); + bindScopeToEmitter(emitter); + + const listener = vi.fn(); + emitter.on('data', listener); + emitter.off('data', listener); + + emitter.emit('data'); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('supports removeAllListeners', () => { + const emitter = new EventEmitter(); + + let boundScope: Scope | undefined; + withScope(scope => { + boundScope = scope; + bindScopeToEmitter(emitter); + }); + + const a = vi.fn(); + const b = vi.fn(); + emitter.on('data', a); + emitter.on('end', b); + + emitter.removeAllListeners('data'); + emitter.emit('data'); + expect(a).not.toHaveBeenCalled(); + + // listeners added after a targeted removeAllListeners are still bound and fire + let scopeInListener: Scope | undefined; + emitter.on('data', () => { + scopeInListener = getCurrentScope(); + }); + emitter.emit('data'); + expect(scopeInListener).toBe(boundScope); + + emitter.removeAllListeners(); + emitter.emit('end'); + expect(b).not.toHaveBeenCalled(); + }); + + it('does not double-wrap when binding the same emitter twice', () => { + const emitter = new EventEmitter(); + + const first = bindScopeToEmitter(emitter); + const second = bindScopeToEmitter(emitter); + + expect(first).toBe(emitter); + expect(second).toBe(emitter); + + const listener = vi.fn(); + emitter.on('data', listener); + emitter.emit('data'); + + // Listener fires exactly once per emit (not multiple times due to double wrapping). + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('preserves the emitter return value for chaining', () => { + const emitter = new EventEmitter(); + bindScopeToEmitter(emitter); + + const result = emitter.on('a', () => {}).on('b', () => {}); + expect(result).toBe(emitter); + }); + + it('passes through objects that are not event emitters', () => { + const obj = { foo: 'bar' }; + expect(bindScopeToEmitter(obj)).toBe(obj); + }); + + describe('DOM EventTarget', () => { + it('binds the scope for listeners added via addEventListener', () => { + const target = new EventTarget(); + + let boundScope: Scope | undefined; + withScope(scope => { + boundScope = scope; + bindScopeToEmitter(target); + }); + + let scopeInListener: Scope | undefined; + target.addEventListener('data', () => { + scopeInListener = getCurrentScope(); + }); + + target.dispatchEvent(new Event('data')); + + expect(scopeInListener).toBe(boundScope); + }); + + it('removes the wrapped listener via removeEventListener', () => { + const target = new EventTarget(); + bindScopeToEmitter(target); + + const listener = vi.fn(); + target.addEventListener('data', listener); + target.removeEventListener('data', listener); + + target.dispatchEvent(new Event('data')); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('forwards the options argument (e.g. `once`)', () => { + const target = new EventTarget(); + bindScopeToEmitter(target); + + const listener = vi.fn(); + target.addEventListener('data', listener, { once: true }); + + target.dispatchEvent(new Event('data')); + target.dispatchEvent(new Event('data')); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('passes through non-function (EventListener object) listeners without throwing', () => { + const target = new EventTarget(); + bindScopeToEmitter(target); + + const handleEvent = vi.fn(); + const listenerObject = { handleEvent }; + target.addEventListener('data', listenerObject); + + expect(() => target.dispatchEvent(new Event('data'))).not.toThrow(); + expect(handleEvent).toHaveBeenCalledTimes(1); + }); + }); + + it('forwards `this` and arguments to the original listener', () => { + const emitter = new EventEmitter(); + bindScopeToEmitter(emitter); + + const listener = vi.fn(); + emitter.on('data', listener); + emitter.emit('data', 1, 'two', { three: true }); + + expect(listener).toHaveBeenCalledWith(1, 'two', { three: true }); + // EventEmitter invokes listeners with `this` bound to the emitter. + expect(listener.mock.instances[0]).toBe(emitter); + }); +}); diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 95890bdc54b9..57b354befd7d 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -65,6 +65,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, // eslint-disable-next-line deprecation/deprecation inboundFiltersIntegration, diff --git a/packages/elysia/src/index.ts b/packages/elysia/src/index.ts index 55542acfc55f..391f221502e8 100644 --- a/packages/elysia/src/index.ts +++ b/packages/elysia/src/index.ts @@ -79,6 +79,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getRootSpan, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 2662ef4720a0..eeba1c6a2e7a 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -80,6 +80,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getRootSpan, diff --git a/packages/node-core/src/common-exports.ts b/packages/node-core/src/common-exports.ts index ddedd5c171eb..205f7cef159d 100644 --- a/packages/node-core/src/common-exports.ts +++ b/packages/node-core/src/common-exports.ts @@ -102,6 +102,7 @@ export { startSpanManual, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, getActiveSpan, withActiveSpan, diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 3bd5e1edba1c..0bd7ccd8d195 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -119,6 +119,7 @@ export { startSpanManual, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, getActiveSpan, withActiveSpan, diff --git a/packages/remix/src/cloudflare/index.ts b/packages/remix/src/cloudflare/index.ts index 97f2609bf10d..641a17775887 100644 --- a/packages/remix/src/cloudflare/index.ts +++ b/packages/remix/src/cloudflare/index.ts @@ -88,6 +88,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getSpanDescendants, diff --git a/packages/remix/src/server/index.ts b/packages/remix/src/server/index.ts index cc5f92bc2948..3e39b42f09df 100644 --- a/packages/remix/src/server/index.ts +++ b/packages/remix/src/server/index.ts @@ -108,6 +108,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, startSession, startSpan, diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts index 06c95b7bbc8b..68f3ca41b7f5 100644 --- a/packages/solidstart/src/server/index.ts +++ b/packages/solidstart/src/server/index.ts @@ -112,6 +112,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, startSession, startSpan, diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 4bd05d6f4657..2c8b9c809418 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -109,6 +109,7 @@ export { spotlightIntegration, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, startSession, startSpan, diff --git a/packages/sveltekit/src/worker/index.ts b/packages/sveltekit/src/worker/index.ts index 89beca6d718f..e421e4c89a43 100644 --- a/packages/sveltekit/src/worker/index.ts +++ b/packages/sveltekit/src/worker/index.ts @@ -72,6 +72,7 @@ export { spanToTraceHeader, startInactiveSpan, startNewTrace, + bindScopeToEmitter, suppressTracing, startSpan, startSpanManual, diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 655ade4e18ad..2310a8190a7a 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -63,6 +63,7 @@ export { startInactiveSpan, startSpanManual, startNewTrace, + bindScopeToEmitter, suppressTracing, withActiveSpan, getSpanDescendants, From 9fcbe97612b17413874887f2de6de6566a0a5fb9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 17 Jun 2026 11:04:49 +0200 Subject: [PATCH 2/5] ref(browser): Exclude bindScopeToEmitter from CDN bundles Keep the API on the npm packages only; CDN bundle size should not grow for a Node-oriented helper. Skip the browser integration test in bundle mode. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../suites/tracing/bindScopeToEmitter/test.ts | 10 ++++++++-- .../browser/src/index.bundle.tracing.logs.metrics.ts | 1 - ...ndex.bundle.tracing.replay.feedback.logs.metrics.ts | 1 - .../src/index.bundle.tracing.replay.feedback.ts | 1 - .../src/index.bundle.tracing.replay.logs.metrics.ts | 1 - packages/browser/src/index.bundle.tracing.replay.ts | 1 - packages/browser/src/index.bundle.tracing.ts | 1 - 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts index 0f4a7bdffc0f..f95829febe1d 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/bindScopeToEmitter/test.ts @@ -1,9 +1,15 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../utils/fixtures'; -import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers'; +import { + envelopeRequestParser, + shouldSkipCdnBundleTest, + shouldSkipTracingTest, + waitForTransactionRequest, +} from '../../../utils/helpers'; sentryTest('bindScopeToEmitter runs listeners with the bound scope active', async ({ getLocalTestUrl, page }) => { - if (shouldSkipTracingTest()) { + // `bindScopeToEmitter` is not exported from the CDN bundles, only from npm. + if (shouldSkipTracingTest() || shouldSkipCdnBundleTest()) { sentryTest.skip(); } diff --git a/packages/browser/src/index.bundle.tracing.logs.metrics.ts b/packages/browser/src/index.bundle.tracing.logs.metrics.ts index 7e9139e153aa..1dd34bb2ff42 100644 --- a/packages/browser/src/index.bundle.tracing.logs.metrics.ts +++ b/packages/browser/src/index.bundle.tracing.logs.metrics.ts @@ -15,7 +15,6 @@ export { setMeasurement, startInactiveSpan, startNewTrace, - bindScopeToEmitter, startSpan, startSpanManual, withActiveSpan, diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts index 355877526bd4..cc7262dc9dbe 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.logs.metrics.ts @@ -13,7 +13,6 @@ export { setMeasurement, startInactiveSpan, startNewTrace, - bindScopeToEmitter, startSpan, startSpanManual, withActiveSpan, diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index 6147eca88328..1e821b38f824 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -20,7 +20,6 @@ export { startInactiveSpan, startSpanManual, startNewTrace, - bindScopeToEmitter, withActiveSpan, getSpanDescendants, setMeasurement, diff --git a/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts b/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts index 96def57ab8e0..9abdd4675d5e 100644 --- a/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts +++ b/packages/browser/src/index.bundle.tracing.replay.logs.metrics.ts @@ -15,7 +15,6 @@ export { setMeasurement, startInactiveSpan, startNewTrace, - bindScopeToEmitter, startSpan, startSpanManual, withActiveSpan, diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index 990d1c3bbf4d..50da2dac87c4 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -20,7 +20,6 @@ export { startInactiveSpan, startSpanManual, startNewTrace, - bindScopeToEmitter, withActiveSpan, getSpanDescendants, setMeasurement, diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index db7de22b9c3e..aac6825ecc65 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -21,7 +21,6 @@ export { startInactiveSpan, startSpanManual, startNewTrace, - bindScopeToEmitter, withActiveSpan, getSpanDescendants, setMeasurement, From c0989175038959ad782f003cc60f732ef0de279b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 17 Jun 2026 11:34:42 +0200 Subject: [PATCH 3/5] fix(core): Track multiple registrations of the same listener in bindScopeToEmitter A listener can be registered for the same event more than once; each is an independent registration in Node's EventEmitter. Storing a single wrapper per (event, listener) meant later registrations overwrote earlier ones, so only the most recent wrapper could be removed via the original reference and earlier ones were orphaned. Keep a stack of wrappers and remove one per removeListener call. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/src/tracing/bindScopeToEmitter.ts | 28 +++++++++---- .../lib/tracing/bindScopeToEmitter.test.ts | 42 +++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/packages/core/src/tracing/bindScopeToEmitter.ts b/packages/core/src/tracing/bindScopeToEmitter.ts index a5b6d75f4a55..86313e7bf9c4 100644 --- a/packages/core/src/tracing/bindScopeToEmitter.ts +++ b/packages/core/src/tracing/bindScopeToEmitter.ts @@ -3,8 +3,13 @@ import type { Scope } from '../scope'; type BoundListener = (...args: unknown[]) => unknown; -/** Per-event map from user-provided listeners to their scope-bound wrappers. */ -type ListenerPatchMap = Record | undefined>; +/** + * Per-event map from a user-provided listener to its scope-bound wrappers. A listener may be + * registered for the same event more than once (each registration is independent in Node's + * `EventEmitter`), so we keep a stack of wrappers rather than a single one — otherwise only the + * most recent registration would be removable via the original listener reference. + */ +type ListenerPatchMap = Record | undefined>; // We patch both Node.js `EventEmitter` registration methods (`on`, `addListener`, ...) and the DOM // `EventTarget.addEventListener`, so this works for Node emitters and browser-native event targets. @@ -111,7 +116,12 @@ function patchAddListener(ee: EventEmitterLike, original: BoundListener, scope: } const boundListener = bindListenerToScope(listener as BoundListener, scope); - listeners.set(listener as BoundListener, boundListener); + const wrappers = listeners.get(listener as BoundListener); + if (wrappers) { + wrappers.push(boundListener); + } else { + listeners.set(listener as BoundListener, [boundListener]); + } isAddingBoundListener = true; try { @@ -128,12 +138,16 @@ function patchRemoveListener(ee: EventEmitterLike, original: BoundListener): Bou const listener = args[1]; const rest = args.slice(2); - const listeners = getPatchMap(ee)?.[event]; - if (!listeners || typeof listener !== 'function') { + const wrappers = + typeof listener === 'function' ? getPatchMap(ee)?.[event]?.get(listener as BoundListener) : undefined; + if (!wrappers?.length) { return original.apply(this, args); } - const boundListener = listeners.get(listener as BoundListener); - return original.call(this, event, boundListener || (listener as BoundListener), ...rest); + // Remove the most recently registered wrapper, mirroring Node's `removeListener` (which removes + // the last-added matching instance). All wrappers for a given listener+scope are equivalent, so + // which one we drop is immaterial — what matters is that each call removes a distinct wrapper. + const boundListener = wrappers.pop() as BoundListener; + return original.call(this, event, boundListener, ...rest); }; } diff --git a/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts b/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts index ba79e1570f4d..df85e9acd9ed 100644 --- a/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts +++ b/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts @@ -151,6 +151,48 @@ describe('bindScopeToEmitter', () => { expect(emitter.listenerCount('data')).toBe(0); }); + it('handles the same listener registered multiple times for one event', () => { + const emitter = new EventEmitter(); + bindScopeToEmitter(emitter); + + const listener = vi.fn(); + emitter.on('data', listener); + emitter.on('data', listener); + expect(emitter.listenerCount('data')).toBe(2); + + emitter.emit('data'); + expect(listener).toHaveBeenCalledTimes(2); + + // Each `removeListener` must remove a distinct registration — neither wrapper may be orphaned. + emitter.removeListener('data', listener); + expect(emitter.listenerCount('data')).toBe(1); + emitter.removeListener('data', listener); + expect(emitter.listenerCount('data')).toBe(0); + + listener.mockClear(); + emitter.emit('data'); + expect(listener).not.toHaveBeenCalled(); + }); + + it('handles a mix of `once` and `on` registrations of the same listener', () => { + const emitter = new EventEmitter(); + bindScopeToEmitter(emitter); + + const listener = vi.fn(); + emitter.once('data', listener); + emitter.on('data', listener); + expect(emitter.listenerCount('data')).toBe(2); + + // First emit fires both; the `once` registration removes itself. + emitter.emit('data'); + expect(listener).toHaveBeenCalledTimes(2); + expect(emitter.listenerCount('data')).toBe(1); + + // The remaining `on` registration is still removable via the original reference. + emitter.removeListener('data', listener); + expect(emitter.listenerCount('data')).toBe(0); + }); + it('removes the wrapped listener via `off`', () => { const emitter = new EventEmitter(); bindScopeToEmitter(emitter); From 5f013517afc3875c95e17df1eb599d046f269f32 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 17 Jun 2026 11:38:51 +0200 Subject: [PATCH 4/5] tweak --- packages/core/src/tracing/bindScopeToEmitter.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/core/src/tracing/bindScopeToEmitter.ts b/packages/core/src/tracing/bindScopeToEmitter.ts index 86313e7bf9c4..b6ec39111636 100644 --- a/packages/core/src/tracing/bindScopeToEmitter.ts +++ b/packages/core/src/tracing/bindScopeToEmitter.ts @@ -95,6 +95,10 @@ function bindListenerToScope(listener: BoundListener, scope: Scope): BoundListen }; } +function isBoundListener(listener: unknown): listener is BoundListener { + return typeof listener === 'function'; +} + function patchAddListener(ee: EventEmitterLike, original: BoundListener, scope: Scope): BoundListener { return function (this: unknown, ...args: unknown[]) { const event = args[0] as string; @@ -104,7 +108,7 @@ function patchAddListener(ee: EventEmitterLike, original: BoundListener, scope: // Pass through anything we can't wrap: re-entrant registrations and non-function listeners // (e.g. `EventListener` objects passed to `addEventListener`). - if (isAddingBoundListener || typeof listener !== 'function') { + if (isAddingBoundListener || !isBoundListener(listener)) { return original.apply(this, args); } @@ -115,12 +119,12 @@ function patchAddListener(ee: EventEmitterLike, original: BoundListener, scope: map[event] = listeners; } - const boundListener = bindListenerToScope(listener as BoundListener, scope); - const wrappers = listeners.get(listener as BoundListener); + const boundListener = bindListenerToScope(listener, scope); + const wrappers = listeners.get(listener); if (wrappers) { wrappers.push(boundListener); } else { - listeners.set(listener as BoundListener, [boundListener]); + listeners.set(listener, [boundListener]); } isAddingBoundListener = true; @@ -138,15 +142,14 @@ function patchRemoveListener(ee: EventEmitterLike, original: BoundListener): Bou const listener = args[1]; const rest = args.slice(2); - const wrappers = - typeof listener === 'function' ? getPatchMap(ee)?.[event]?.get(listener as BoundListener) : undefined; + const wrappers = isBoundListener(listener) ? getPatchMap(ee)?.[event]?.get(listener) : undefined; if (!wrappers?.length) { return original.apply(this, args); } // Remove the most recently registered wrapper, mirroring Node's `removeListener` (which removes // the last-added matching instance). All wrappers for a given listener+scope are equivalent, so // which one we drop is immaterial — what matters is that each call removes a distinct wrapper. - const boundListener = wrappers.pop() as BoundListener; + const boundListener = wrappers.pop(); return original.call(this, event, boundListener, ...rest); }; } From 60891cf7fe936cd03f71caf4de05c830002dbd4c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 17 Jun 2026 16:36:49 +0200 Subject: [PATCH 5/5] fix(core): Reuse one wrapper per listener in bindScopeToEmitter Minting a fresh wrapper on every registration broke DOM EventTarget semantics: addEventListener dedupes by (type, callback, capture), so distinct wrapper refs defeated that idempotency (the listener fired once per call) and capture-aware removal could drop the wrong wrapper. Reuse a single stable wrapper per listener and forward the caller's options unchanged: the DOM dedupes correctly and Node's EventEmitter still counts duplicate registrations (removable one-per-call). This also subsumes the earlier per-event wrapper stack. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/src/tracing/bindScopeToEmitter.ts | 35 ++++++++++--------- .../lib/tracing/bindScopeToEmitter.test.ts | 31 ++++++++++++++++ 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/packages/core/src/tracing/bindScopeToEmitter.ts b/packages/core/src/tracing/bindScopeToEmitter.ts index b6ec39111636..28225618123a 100644 --- a/packages/core/src/tracing/bindScopeToEmitter.ts +++ b/packages/core/src/tracing/bindScopeToEmitter.ts @@ -4,12 +4,15 @@ import type { Scope } from '../scope'; type BoundListener = (...args: unknown[]) => unknown; /** - * Per-event map from a user-provided listener to its scope-bound wrappers. A listener may be - * registered for the same event more than once (each registration is independent in Node's - * `EventEmitter`), so we keep a stack of wrappers rather than a single one — otherwise only the - * most recent registration would be removable via the original listener reference. + * Per-event map from a user-provided listener to its single scope-bound wrapper. We reuse one stable + * wrapper per listener (rather than minting a new one per registration) and let the underlying + * emitter/target handle repeat registrations: + * - Node's `EventEmitter` allows duplicates and counts them, so registering the same wrapper N times + * fires N times and `removeListener` removes one instance per call — no orphaned wrappers. + * - the DOM `EventTarget` dedupes by `(type, callback, capture)`, so reusing the wrapper preserves + * that idempotency; a fresh wrapper per call would defeat it and fire the listener repeatedly. */ -type ListenerPatchMap = Record | undefined>; +type ListenerPatchMap = Record | undefined>; // We patch both Node.js `EventEmitter` registration methods (`on`, `addListener`, ...) and the DOM // `EventTarget.addEventListener`, so this works for Node emitters and browser-native event targets. @@ -119,12 +122,12 @@ function patchAddListener(ee: EventEmitterLike, original: BoundListener, scope: map[event] = listeners; } - const boundListener = bindListenerToScope(listener, scope); - const wrappers = listeners.get(listener); - if (wrappers) { - wrappers.push(boundListener); - } else { - listeners.set(listener, [boundListener]); + // Reuse one stable wrapper per listener so repeat registrations are handled correctly by the + // underlying emitter/target (Node counts duplicates; the DOM dedupes by `(callback, capture)`). + let boundListener = listeners.get(listener); + if (!boundListener) { + boundListener = bindListenerToScope(listener, scope); + listeners.set(listener, boundListener); } isAddingBoundListener = true; @@ -142,14 +145,12 @@ function patchRemoveListener(ee: EventEmitterLike, original: BoundListener): Bou const listener = args[1]; const rest = args.slice(2); - const wrappers = isBoundListener(listener) ? getPatchMap(ee)?.[event]?.get(listener) : undefined; - if (!wrappers?.length) { + const boundListener = isBoundListener(listener) ? getPatchMap(ee)?.[event]?.get(listener) : undefined; + if (!boundListener) { return original.apply(this, args); } - // Remove the most recently registered wrapper, mirroring Node's `removeListener` (which removes - // the last-added matching instance). All wrappers for a given listener+scope are equivalent, so - // which one we drop is immaterial — what matters is that each call removes a distinct wrapper. - const boundListener = wrappers.pop(); + // Pass the same stable wrapper and forward the caller's extra args (e.g. the `capture` option of + // `removeEventListener`) unchanged, so the emitter/target matches the right registration itself. return original.call(this, event, boundListener, ...rest); }; } diff --git a/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts b/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts index df85e9acd9ed..e5aaea783ddb 100644 --- a/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts +++ b/packages/core/test/lib/tracing/bindScopeToEmitter.test.ts @@ -313,6 +313,37 @@ describe('bindScopeToEmitter', () => { expect(listener).toHaveBeenCalledTimes(1); }); + it('preserves addEventListener idempotency for identical (type, listener) registrations', () => { + const target = new EventTarget(); + bindScopeToEmitter(target); + + const listener = vi.fn(); + // The DOM dedupes identical registrations -> the listener must only fire once. + target.addEventListener('data', listener); + target.addEventListener('data', listener); + + target.dispatchEvent(new Event('data')); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('removes the correct registration when only the capture phase differs', () => { + const target = new EventTarget(); + bindScopeToEmitter(target); + + const listener = vi.fn(); + // Capture is part of a registration's identity, so these are two distinct registrations. + target.addEventListener('data', listener, { capture: true }); + target.addEventListener('data', listener, { capture: false }); + + // Remove only the capture-phase one; the bubble-phase registration must survive. + target.removeEventListener('data', listener, { capture: true }); + + target.dispatchEvent(new Event('data')); + + expect(listener).toHaveBeenCalledTimes(1); + }); + it('passes through non-function (EventListener object) listeners without throwing', () => { const target = new EventTarget(); bindScopeToEmitter(target);