diff --git a/.size-limit.js b/.size-limit.js index cad516a0a49a..3417895018f1 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -59,7 +59,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/prod/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '83 KB', + limit: '84 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', @@ -138,7 +138,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/prod/index.js', import: createImport('init', 'metrics', 'logger'), gzip: true, - limit: '28 KB', + limit: '29 KB', }, // React SDK (ESM) { @@ -191,13 +191,13 @@ module.exports = [ name: 'CDN Bundle (incl. Tracing)', path: createCDNPath('bundle.tracing.min.js'), gzip: true, - limit: '46.5 KB', + limit: '47 KB', }, { name: 'CDN Bundle (incl. Logs, Metrics)', path: createCDNPath('bundle.logs.metrics.min.js'), gzip: true, - limit: '30 KB', + limit: '31 KB', }, { name: 'CDN Bundle (incl. Tracing, Logs, Metrics)', @@ -209,19 +209,19 @@ module.exports = [ name: 'CDN Bundle (incl. Replay, Logs, Metrics)', path: createCDNPath('bundle.replay.logs.metrics.min.js'), gzip: true, - limit: '69 KB', + limit: '70 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay)', path: createCDNPath('bundle.tracing.replay.min.js'), gzip: true, - limit: '83.5 KB', + limit: '84 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay, Logs, Metrics)', path: createCDNPath('bundle.tracing.replay.logs.metrics.min.js'), gzip: true, - limit: '84.5 KB', + limit: '85 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay, Feedback)', @@ -283,21 +283,21 @@ module.exports = [ path: createCDNPath('bundle.tracing.replay.logs.metrics.min.js'), gzip: false, brotli: false, - limit: '258.5 KB', + limit: '259 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed', path: createCDNPath('bundle.tracing.replay.feedback.min.js'), gzip: false, brotli: false, - limit: '268 KB', + limit: '269 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed', path: createCDNPath('bundle.tracing.replay.feedback.logs.metrics.min.js'), gzip: false, brotli: false, - limit: '271.5 KB', + limit: '272 KB', }, // Next.js SDK (ESM) { diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 00c12db06855..5b3aac28d93e 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -212,6 +212,14 @@ export abstract class Client { protected _promiseBuffer: PromiseBuffer; + /** + * Cleanup functions to call on dispose. + * + * NOTE: These callbacks are only invoked by subclasses whose `dispose()` implementation runs them + * (currently only `ServerRuntimeClient`). The base `Client.dispose()` is a no-op and will not run them. + */ + protected _disposeCallbacks: (() => void)[]; + /** * Initializes this client instance. * @@ -224,6 +232,7 @@ export abstract class Client { this._outcomes = {}; this._hooks = {}; this._eventProcessors = []; + this._disposeCallbacks = []; this._promiseBuffer = makePromiseBuffer(options.transportOptions?.bufferSize ?? DEFAULT_TRANSPORT_BUFFER_SIZE); if (options.dsn) { @@ -1169,10 +1178,26 @@ export abstract class Client { return {}; } + /** + * Register a cleanup function to be called when the client is disposed. + * This is useful for integrations that need to clean up global state. + * + * NOTE: Registered callbacks are only executed by subclasses whose `dispose()` implementation + * runs them. At the moment that is only `ServerRuntimeClient` (and clients extending it). On the + * base `Client` (e.g. the browser client), `dispose()` is a no-op, so callbacks registered here + * will never be invoked. + */ + public registerCleanup(callback: () => void): void { + this._disposeCallbacks.push(callback); + } + /** * Disposes of the client and releases all resources. * - * Subclasses should override this method to clean up their own resources. + * Subclasses should override this method to clean up their own resources, including invoking + * any callbacks registered via {@link Client.registerCleanup}. The base implementation is a + * no-op and does NOT execute registered cleanup callbacks. + * * After calling dispose(), the client should not be used anymore. */ public dispose(): void { diff --git a/packages/core/src/instrument/console.ts b/packages/core/src/instrument/console.ts index cecf1e5cad8a..ef7e9c804943 100644 --- a/packages/core/src/instrument/console.ts +++ b/packages/core/src/instrument/console.ts @@ -8,14 +8,16 @@ import { addHandler, maybeInstrument, triggerHandlers } from './handlers'; /** * Add an instrumentation handler for when a console.xxx method is called. + * Returns a function to remove the handler. * * Use at your own risk, this might break without changelog notice, only used internally. * @hidden */ -export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): void { +export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): () => void { const type = 'console'; - addHandler(type, handler); + const removeHandler = addHandler(type, handler); maybeInstrument(type, instrumentConsole); + return removeHandler; } function instrumentConsole(): void { diff --git a/packages/core/src/instrument/fetch.ts b/packages/core/src/instrument/fetch.ts index 590830ab4e20..a3165cfbc13a 100644 --- a/packages/core/src/instrument/fetch.ts +++ b/packages/core/src/instrument/fetch.ts @@ -15,6 +15,7 @@ type FetchResource = string | { toString(): string } | { url: string }; * Add an instrumentation handler for when a fetch request happens. * The handler function is called once when the request starts and once when it ends, * which can be identified by checking if it has an `endTimestamp`. + * Returns a function to remove the handler. * * Use at your own risk, this might break without changelog notice, only used internally. * @hidden @@ -22,24 +23,27 @@ type FetchResource = string | { toString(): string } | { url: string }; export function addFetchInstrumentationHandler( handler: (data: HandlerDataFetch) => void, skipNativeFetchCheck?: boolean, -): void { +): () => void { const type = 'fetch'; - addHandler(type, handler); + const removeHandler = addHandler(type, handler); maybeInstrument(type, () => instrumentFetch(undefined, skipNativeFetchCheck)); + return removeHandler; } /** * Add an instrumentation handler for long-lived fetch requests, like consuming server-sent events (SSE) via fetch. * The handler will resolve the request body and emit the actual `endTimestamp`, so that the * span can be updated accordingly. + * Returns a function to remove the handler. * * Only used internally * @hidden */ -export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFetch) => void): void { +export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFetch) => void): () => void { const type = 'fetch-body-resolved'; - addHandler(type, handler); + const removeHandler = addHandler(type, handler); maybeInstrument(type, () => instrumentFetch(streamHandler)); + return removeHandler; } function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNativeFetchCheck: boolean = false): void { diff --git a/packages/core/src/instrument/handlers.ts b/packages/core/src/instrument/handlers.ts index 74dbc9902348..bd15263021c8 100644 --- a/packages/core/src/instrument/handlers.ts +++ b/packages/core/src/instrument/handlers.ts @@ -18,10 +18,20 @@ export type InstrumentHandlerCallback = (data: any) => void; const handlers: { [key in InstrumentHandlerType]?: InstrumentHandlerCallback[] } = {}; const instrumented: { [key in InstrumentHandlerType]?: boolean } = {}; -/** Add a handler function. */ -export function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallback): void { +/** Add a handler function. Returns a function to remove the handler. */ +export function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallback): () => void { handlers[type] = handlers[type] || []; handlers[type].push(handler); + + return () => { + const typeHandlers = handlers[type]; + if (typeHandlers) { + const index = typeHandlers.indexOf(handler); + if (index !== -1) { + typeHandlers.splice(index, 1); + } + } + }; } /** diff --git a/packages/core/src/integrations/console.ts b/packages/core/src/integrations/console.ts index dda44543cc03..e39fd5ddcf0d 100644 --- a/packages/core/src/integrations/console.ts +++ b/packages/core/src/integrations/console.ts @@ -41,13 +41,15 @@ export const consoleIntegration = defineIntegration((options: Partial { + const unsubscribe = addConsoleInstrumentationHandler(({ args, level }) => { if (getClient() !== client || !levels.has(level)) { return; } addConsoleBreadcrumb(level, args); }); + + client.registerCleanup(unsubscribe); }, }; }); diff --git a/packages/core/src/integrations/conversationId.ts b/packages/core/src/integrations/conversationId.ts index 445e3327419b..36a3f91d3234 100644 --- a/packages/core/src/integrations/conversationId.ts +++ b/packages/core/src/integrations/conversationId.ts @@ -12,7 +12,7 @@ const _conversationIdIntegration = (() => { return { name: INTEGRATION_NAME, setup(client: Client) { - client.on('spanStart', (span: Span) => { + const unsubscribe = client.on('spanStart', (span: Span) => { const scopeData = getCurrentScope().getScopeData(); const isolationScopeData = getIsolationScope().getScopeData(); @@ -32,6 +32,8 @@ const _conversationIdIntegration = (() => { span.setAttribute(GEN_AI_CONVERSATION_ID_ATTRIBUTE, conversationId); } }); + + client.registerCleanup(unsubscribe); }, }; }) satisfies IntegrationFn; diff --git a/packages/core/src/logs/console-integration.ts b/packages/core/src/logs/console-integration.ts index ccf14e3ebf48..e16016a1154a 100644 --- a/packages/core/src/logs/console-integration.ts +++ b/packages/core/src/logs/console-integration.ts @@ -31,7 +31,7 @@ const _consoleLoggingIntegration = ((options: Partial = { return; } - addConsoleInstrumentationHandler(({ args, level }) => { + const unsubscribe = addConsoleInstrumentationHandler(({ args, level }) => { if (getClient() !== client || !levels.includes(level)) { return; } @@ -66,6 +66,8 @@ const _consoleLoggingIntegration = ((options: Partial = { attributes, }); }); + + client.registerCleanup(unsubscribe); }, }; }) satisfies IntegrationFn; diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index a1958f0bcbbb..030dfef92477 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -168,6 +168,16 @@ export class ServerRuntimeClient< public override dispose(): void { DEBUG_BUILD && debug.log('Disposing client...'); + // Run all registered cleanup callbacks + for (const callback of this._disposeCallbacks) { + try { + callback(); + } catch { + // Ignore errors in cleanup callbacks + } + } + this._disposeCallbacks.length = 0; + for (const hookName of Object.keys(this._hooks)) { this._hooks[hookName]?.clear(); } diff --git a/packages/core/test/lib/instrument/handlers.test.ts b/packages/core/test/lib/instrument/handlers.test.ts index 87e227a99323..cb894514b24a 100644 --- a/packages/core/test/lib/instrument/handlers.test.ts +++ b/packages/core/test/lib/instrument/handlers.test.ts @@ -1,5 +1,14 @@ -import { describe, test } from 'vitest'; -import { maybeInstrument } from '../../../src/instrument/handlers'; +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { + addHandler, + maybeInstrument, + resetInstrumentationHandlers, + triggerHandlers, +} from '../../../src/instrument/handlers'; + +afterEach(() => { + resetInstrumentationHandlers(); +}); describe('maybeInstrument', () => { test('does not throw when instrumenting fails', () => { @@ -12,3 +21,89 @@ describe('maybeInstrument', () => { maybeInstrument('xhr', undefined as any); }); }); + +describe('addHandler', () => { + test('returns an unsubscribe function', () => { + const handler = vi.fn(); + const unsubscribe = addHandler('fetch', handler); + + expect(typeof unsubscribe).toBe('function'); + }); + + test('handler is called when triggerHandlers is invoked', () => { + const handler = vi.fn(); + addHandler('fetch', handler); + + triggerHandlers('fetch', { url: 'https://example.com' }); + + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith({ url: 'https://example.com' }); + }); + + test('unsubscribe removes the handler', () => { + const handler = vi.fn(); + const unsubscribe = addHandler('fetch', handler); + + triggerHandlers('fetch', { test: 1 }); + expect(handler).toHaveBeenCalledTimes(1); + + unsubscribe(); + + triggerHandlers('fetch', { test: 2 }); + expect(handler).toHaveBeenCalledTimes(1); + }); + + test('unsubscribe only removes the specific handler', () => { + const handler1 = vi.fn(); + const handler2 = vi.fn(); + + const unsubscribe1 = addHandler('fetch', handler1); + addHandler('fetch', handler2); + + triggerHandlers('fetch', { test: 1 }); + expect(handler1).toHaveBeenCalledTimes(1); + expect(handler2).toHaveBeenCalledTimes(1); + + unsubscribe1(); + + triggerHandlers('fetch', { test: 2 }); + expect(handler1).toHaveBeenCalledTimes(1); + expect(handler2).toHaveBeenCalledTimes(2); + }); + + test('calling unsubscribe multiple times is safe', () => { + const handler = vi.fn(); + const unsubscribe = addHandler('fetch', handler); + + unsubscribe(); + expect(() => unsubscribe()).not.toThrow(); + expect(() => unsubscribe()).not.toThrow(); + }); + + test('unsubscribe works with different handler types', () => { + const consoleHandler = vi.fn(); + const fetchHandler = vi.fn(); + + const unsubscribeConsole = addHandler('console', consoleHandler); + const unsubscribeFetch = addHandler('fetch', fetchHandler); + + triggerHandlers('console', { level: 'log' }); + triggerHandlers('fetch', { url: 'test' }); + + expect(consoleHandler).toHaveBeenCalledTimes(1); + expect(fetchHandler).toHaveBeenCalledTimes(1); + + unsubscribeConsole(); + + triggerHandlers('console', { level: 'warn' }); + triggerHandlers('fetch', { url: 'test2' }); + + expect(consoleHandler).toHaveBeenCalledTimes(1); + expect(fetchHandler).toHaveBeenCalledTimes(2); + + unsubscribeFetch(); + + triggerHandlers('fetch', { url: 'test3' }); + expect(fetchHandler).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/core/test/lib/server-runtime-client.test.ts b/packages/core/test/lib/server-runtime-client.test.ts index bbe9ee84a716..8d7f7fbc48c5 100644 --- a/packages/core/test/lib/server-runtime-client.test.ts +++ b/packages/core/test/lib/server-runtime-client.test.ts @@ -320,5 +320,90 @@ describe('ServerRuntimeClient', () => { // Verify it's a fresh buffer with no pending items expect(bufferAfterDispose.$).toEqual([]); }); + + it('calls registered cleanup callbacks on dispose', () => { + const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); + client = new ServerRuntimeClient(options); + + const cleanup1 = vi.fn(); + const cleanup2 = vi.fn(); + const cleanup3 = vi.fn(); + + client.registerCleanup(cleanup1); + client.registerCleanup(cleanup2); + client.registerCleanup(cleanup3); + + expect(cleanup1).not.toHaveBeenCalled(); + expect(cleanup2).not.toHaveBeenCalled(); + expect(cleanup3).not.toHaveBeenCalled(); + + client.dispose(); + + expect(cleanup1).toHaveBeenCalledTimes(1); + expect(cleanup2).toHaveBeenCalledTimes(1); + expect(cleanup3).toHaveBeenCalledTimes(1); + }); + + it('clears cleanup callbacks after dispose', () => { + const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); + client = new ServerRuntimeClient(options); + + const cleanup = vi.fn(); + client.registerCleanup(cleanup); + + client.dispose(); + expect(cleanup).toHaveBeenCalledTimes(1); + + // Calling dispose again should not call cleanup again + client.dispose(); + expect(cleanup).toHaveBeenCalledTimes(1); + }); + + it('continues to call other cleanup callbacks if one throws', () => { + const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); + client = new ServerRuntimeClient(options); + + const cleanup1 = vi.fn(); + const throwingCleanup = vi.fn(() => { + throw new Error('cleanup error'); + }); + const cleanup2 = vi.fn(); + + client.registerCleanup(cleanup1); + client.registerCleanup(throwingCleanup); + client.registerCleanup(cleanup2); + + expect(() => client.dispose()).not.toThrow(); + + expect(cleanup1).toHaveBeenCalledTimes(1); + expect(throwingCleanup).toHaveBeenCalledTimes(1); + expect(cleanup2).toHaveBeenCalledTimes(1); + }); + }); + + describe('registerCleanup', () => { + it('accepts cleanup functions', () => { + const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); + client = new ServerRuntimeClient(options); + + const cleanup = vi.fn(); + + expect(() => client.registerCleanup(cleanup)).not.toThrow(); + }); + + it('can register multiple cleanup functions', () => { + const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); + client = new ServerRuntimeClient(options); + + const cleanups = Array.from({ length: 10 }, () => vi.fn()); + + cleanups.forEach(cleanup => client.registerCleanup(cleanup)); + + client.dispose(); + + cleanups.forEach(cleanup => { + expect(cleanup).toHaveBeenCalledTimes(1); + }); + }); }); }); diff --git a/packages/node-core/test/integrations/console.test.ts b/packages/node-core/test/integrations/console.test.ts index 0355fe2d076b..39086e9768e3 100644 --- a/packages/node-core/test/integrations/console.test.ts +++ b/packages/node-core/test/integrations/console.test.ts @@ -25,7 +25,7 @@ describe('consoleIntegration in Lambda (patchWithDefineProperty)', () => { it('calls registered handler when console.log is called', () => { const handler = vi.fn(); // Setup the integration so it calls maybeInstrument with the Lambda strategy - consoleIntegration().setup?.({ on: vi.fn() } as any); + consoleIntegration().setup?.({ on: vi.fn(), registerCleanup: vi.fn() } as any); addConsoleInstrumentationHandler(handler);