Skip to content

Commit 04e05e8

Browse files
committed
feat(core): clear up integrations on dispose
1 parent be13537 commit 04e05e8

10 files changed

Lines changed: 237 additions & 13 deletions

File tree

packages/core/src/client.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
211211

212212
protected _promiseBuffer: PromiseBuffer<unknown>;
213213

214+
/** Cleanup functions to call on dispose */
215+
protected _disposeCallbacks: (() => void)[];
216+
214217
/**
215218
* Initializes this client instance.
216219
*
@@ -223,6 +226,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
223226
this._outcomes = {};
224227
this._hooks = {};
225228
this._eventProcessors = [];
229+
this._disposeCallbacks = [];
226230
this._promiseBuffer = makePromiseBuffer(options.transportOptions?.bufferSize ?? DEFAULT_TRANSPORT_BUFFER_SIZE);
227231

228232
if (options.dsn) {
@@ -1145,6 +1149,14 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
11451149
return {};
11461150
}
11471151

1152+
/**
1153+
* Register a cleanup function to be called when the client is disposed.
1154+
* This is useful for integrations that need to clean up global state.
1155+
*/
1156+
public registerCleanup(callback: () => void): void {
1157+
this._disposeCallbacks.push(callback);
1158+
}
1159+
11481160
/**
11491161
* Disposes of the client and releases all resources.
11501162
*

packages/core/src/instrument/console.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,16 @@ import { addHandler, maybeInstrument, triggerHandlers } from './handlers';
88

99
/**
1010
* Add an instrumentation handler for when a console.xxx method is called.
11+
* Returns a function to remove the handler.
1112
*
1213
* Use at your own risk, this might break without changelog notice, only used internally.
1314
* @hidden
1415
*/
15-
export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): void {
16+
export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): () => void {
1617
const type = 'console';
17-
addHandler(type, handler);
18+
const removeHandler = addHandler(type, handler);
1819
maybeInstrument(type, instrumentConsole);
20+
return removeHandler;
1921
}
2022

2123
function instrumentConsole(): void {

packages/core/src/instrument/fetch.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,31 +15,35 @@ type FetchResource = string | { toString(): string } | { url: string };
1515
* Add an instrumentation handler for when a fetch request happens.
1616
* The handler function is called once when the request starts and once when it ends,
1717
* which can be identified by checking if it has an `endTimestamp`.
18+
* Returns a function to remove the handler.
1819
*
1920
* Use at your own risk, this might break without changelog notice, only used internally.
2021
* @hidden
2122
*/
2223
export function addFetchInstrumentationHandler(
2324
handler: (data: HandlerDataFetch) => void,
2425
skipNativeFetchCheck?: boolean,
25-
): void {
26+
): () => void {
2627
const type = 'fetch';
27-
addHandler(type, handler);
28+
const removeHandler = addHandler(type, handler);
2829
maybeInstrument(type, () => instrumentFetch(undefined, skipNativeFetchCheck));
30+
return removeHandler;
2931
}
3032

3133
/**
3234
* Add an instrumentation handler for long-lived fetch requests, like consuming server-sent events (SSE) via fetch.
3335
* The handler will resolve the request body and emit the actual `endTimestamp`, so that the
3436
* span can be updated accordingly.
37+
* Returns a function to remove the handler.
3538
*
3639
* Only used internally
3740
* @hidden
3841
*/
39-
export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFetch) => void): void {
42+
export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFetch) => void): () => void {
4043
const type = 'fetch-body-resolved';
41-
addHandler(type, handler);
44+
const removeHandler = addHandler(type, handler);
4245
maybeInstrument(type, () => instrumentFetch(streamHandler));
46+
return removeHandler;
4347
}
4448

4549
function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNativeFetchCheck: boolean = false): void {

packages/core/src/instrument/handlers.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,20 @@ export type InstrumentHandlerCallback = (data: any) => void;
1818
const handlers: { [key in InstrumentHandlerType]?: InstrumentHandlerCallback[] } = {};
1919
const instrumented: { [key in InstrumentHandlerType]?: boolean } = {};
2020

21-
/** Add a handler function. */
22-
export function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallback): void {
21+
/** Add a handler function. Returns a function to remove the handler. */
22+
export function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallback): () => void {
2323
handlers[type] = handlers[type] || [];
2424
handlers[type].push(handler);
25+
26+
return () => {
27+
const typeHandlers = handlers[type];
28+
if (typeHandlers) {
29+
const index = typeHandlers.indexOf(handler);
30+
if (index !== -1) {
31+
typeHandlers.splice(index, 1);
32+
}
33+
}
34+
};
2535
}
2636

2737
/**

packages/core/src/integrations/console.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,15 @@ export const consoleIntegration = defineIntegration((options: Partial<ConsoleInt
4141
return {
4242
name: INTEGRATION_NAME,
4343
setup(client) {
44-
addConsoleInstrumentationHandler(({ args, level }) => {
44+
const unsubscribe = addConsoleInstrumentationHandler(({ args, level }) => {
4545
if (getClient() !== client || !levels.has(level)) {
4646
return;
4747
}
4848

4949
addConsoleBreadcrumb(level, args);
5050
});
51+
52+
client.registerCleanup(unsubscribe);
5153
},
5254
};
5355
});

packages/core/src/integrations/conversationId.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const _conversationIdIntegration = (() => {
1212
return {
1313
name: INTEGRATION_NAME,
1414
setup(client: Client) {
15-
client.on('spanStart', (span: Span) => {
15+
const unsubscribe = client.on('spanStart', (span: Span) => {
1616
const scopeData = getCurrentScope().getScopeData();
1717
const isolationScopeData = getIsolationScope().getScopeData();
1818

@@ -32,6 +32,8 @@ const _conversationIdIntegration = (() => {
3232
span.setAttribute(GEN_AI_CONVERSATION_ID_ATTRIBUTE, conversationId);
3333
}
3434
});
35+
36+
client.registerCleanup(unsubscribe);
3537
},
3638
};
3739
}) satisfies IntegrationFn;

packages/core/src/logs/console-integration.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const _consoleLoggingIntegration = ((options: Partial<CaptureConsoleOptions> = {
3131
return;
3232
}
3333

34-
addConsoleInstrumentationHandler(({ args, level }) => {
34+
const unsubscribe = addConsoleInstrumentationHandler(({ args, level }) => {
3535
if (getClient() !== client || !levels.includes(level)) {
3636
return;
3737
}
@@ -66,6 +66,8 @@ const _consoleLoggingIntegration = ((options: Partial<CaptureConsoleOptions> = {
6666
attributes,
6767
});
6868
});
69+
70+
client.registerCleanup(unsubscribe);
6971
},
7072
};
7173
}) satisfies IntegrationFn;

packages/core/src/server-runtime-client.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,16 @@ export class ServerRuntimeClient<
168168
public override dispose(): void {
169169
DEBUG_BUILD && debug.log('Disposing client...');
170170

171+
// Run all registered cleanup callbacks
172+
for (const callback of this._disposeCallbacks) {
173+
try {
174+
callback();
175+
} catch {
176+
// Ignore errors in cleanup callbacks
177+
}
178+
}
179+
this._disposeCallbacks.length = 0;
180+
171181
for (const hookName of Object.keys(this._hooks)) {
172182
this._hooks[hookName]?.clear();
173183
}

packages/core/test/lib/instrument/handlers.test.ts

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
1-
import { describe, test } from 'vitest';
2-
import { maybeInstrument } from '../../../src/instrument/handlers';
1+
import { afterEach, describe, expect, test, vi } from 'vitest';
2+
import {
3+
addHandler,
4+
maybeInstrument,
5+
resetInstrumentationHandlers,
6+
triggerHandlers,
7+
} from '../../../src/instrument/handlers';
8+
9+
afterEach(() => {
10+
resetInstrumentationHandlers();
11+
});
312

413
describe('maybeInstrument', () => {
514
test('does not throw when instrumenting fails', () => {
@@ -12,3 +21,89 @@ describe('maybeInstrument', () => {
1221
maybeInstrument('xhr', undefined as any);
1322
});
1423
});
24+
25+
describe('addHandler', () => {
26+
test('returns an unsubscribe function', () => {
27+
const handler = vi.fn();
28+
const unsubscribe = addHandler('fetch', handler);
29+
30+
expect(typeof unsubscribe).toBe('function');
31+
});
32+
33+
test('handler is called when triggerHandlers is invoked', () => {
34+
const handler = vi.fn();
35+
addHandler('fetch', handler);
36+
37+
triggerHandlers('fetch', { url: 'https://example.com' });
38+
39+
expect(handler).toHaveBeenCalledTimes(1);
40+
expect(handler).toHaveBeenCalledWith({ url: 'https://example.com' });
41+
});
42+
43+
test('unsubscribe removes the handler', () => {
44+
const handler = vi.fn();
45+
const unsubscribe = addHandler('fetch', handler);
46+
47+
triggerHandlers('fetch', { test: 1 });
48+
expect(handler).toHaveBeenCalledTimes(1);
49+
50+
unsubscribe();
51+
52+
triggerHandlers('fetch', { test: 2 });
53+
expect(handler).toHaveBeenCalledTimes(1);
54+
});
55+
56+
test('unsubscribe only removes the specific handler', () => {
57+
const handler1 = vi.fn();
58+
const handler2 = vi.fn();
59+
60+
const unsubscribe1 = addHandler('fetch', handler1);
61+
addHandler('fetch', handler2);
62+
63+
triggerHandlers('fetch', { test: 1 });
64+
expect(handler1).toHaveBeenCalledTimes(1);
65+
expect(handler2).toHaveBeenCalledTimes(1);
66+
67+
unsubscribe1();
68+
69+
triggerHandlers('fetch', { test: 2 });
70+
expect(handler1).toHaveBeenCalledTimes(1);
71+
expect(handler2).toHaveBeenCalledTimes(2);
72+
});
73+
74+
test('calling unsubscribe multiple times is safe', () => {
75+
const handler = vi.fn();
76+
const unsubscribe = addHandler('fetch', handler);
77+
78+
unsubscribe();
79+
expect(() => unsubscribe()).not.toThrow();
80+
expect(() => unsubscribe()).not.toThrow();
81+
});
82+
83+
test('unsubscribe works with different handler types', () => {
84+
const consoleHandler = vi.fn();
85+
const fetchHandler = vi.fn();
86+
87+
const unsubscribeConsole = addHandler('console', consoleHandler);
88+
const unsubscribeFetch = addHandler('fetch', fetchHandler);
89+
90+
triggerHandlers('console', { level: 'log' });
91+
triggerHandlers('fetch', { url: 'test' });
92+
93+
expect(consoleHandler).toHaveBeenCalledTimes(1);
94+
expect(fetchHandler).toHaveBeenCalledTimes(1);
95+
96+
unsubscribeConsole();
97+
98+
triggerHandlers('console', { level: 'warn' });
99+
triggerHandlers('fetch', { url: 'test2' });
100+
101+
expect(consoleHandler).toHaveBeenCalledTimes(1);
102+
expect(fetchHandler).toHaveBeenCalledTimes(2);
103+
104+
unsubscribeFetch();
105+
106+
triggerHandlers('fetch', { url: 'test3' });
107+
expect(fetchHandler).toHaveBeenCalledTimes(2);
108+
});
109+
});

packages/core/test/lib/server-runtime-client.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,5 +320,90 @@ describe('ServerRuntimeClient', () => {
320320
// Verify it's a fresh buffer with no pending items
321321
expect(bufferAfterDispose.$).toEqual([]);
322322
});
323+
324+
it('calls registered cleanup callbacks on dispose', () => {
325+
const options = getDefaultClientOptions({ dsn: PUBLIC_DSN });
326+
client = new ServerRuntimeClient(options);
327+
328+
const cleanup1 = vi.fn();
329+
const cleanup2 = vi.fn();
330+
const cleanup3 = vi.fn();
331+
332+
client.registerCleanup(cleanup1);
333+
client.registerCleanup(cleanup2);
334+
client.registerCleanup(cleanup3);
335+
336+
expect(cleanup1).not.toHaveBeenCalled();
337+
expect(cleanup2).not.toHaveBeenCalled();
338+
expect(cleanup3).not.toHaveBeenCalled();
339+
340+
client.dispose();
341+
342+
expect(cleanup1).toHaveBeenCalledTimes(1);
343+
expect(cleanup2).toHaveBeenCalledTimes(1);
344+
expect(cleanup3).toHaveBeenCalledTimes(1);
345+
});
346+
347+
it('clears cleanup callbacks after dispose', () => {
348+
const options = getDefaultClientOptions({ dsn: PUBLIC_DSN });
349+
client = new ServerRuntimeClient(options);
350+
351+
const cleanup = vi.fn();
352+
client.registerCleanup(cleanup);
353+
354+
client.dispose();
355+
expect(cleanup).toHaveBeenCalledTimes(1);
356+
357+
// Calling dispose again should not call cleanup again
358+
client.dispose();
359+
expect(cleanup).toHaveBeenCalledTimes(1);
360+
});
361+
362+
it('continues to call other cleanup callbacks if one throws', () => {
363+
const options = getDefaultClientOptions({ dsn: PUBLIC_DSN });
364+
client = new ServerRuntimeClient(options);
365+
366+
const cleanup1 = vi.fn();
367+
const throwingCleanup = vi.fn(() => {
368+
throw new Error('cleanup error');
369+
});
370+
const cleanup2 = vi.fn();
371+
372+
client.registerCleanup(cleanup1);
373+
client.registerCleanup(throwingCleanup);
374+
client.registerCleanup(cleanup2);
375+
376+
expect(() => client.dispose()).not.toThrow();
377+
378+
expect(cleanup1).toHaveBeenCalledTimes(1);
379+
expect(throwingCleanup).toHaveBeenCalledTimes(1);
380+
expect(cleanup2).toHaveBeenCalledTimes(1);
381+
});
382+
});
383+
384+
describe('registerCleanup', () => {
385+
it('accepts cleanup functions', () => {
386+
const options = getDefaultClientOptions({ dsn: PUBLIC_DSN });
387+
client = new ServerRuntimeClient(options);
388+
389+
const cleanup = vi.fn();
390+
391+
expect(() => client.registerCleanup(cleanup)).not.toThrow();
392+
});
393+
394+
it('can register multiple cleanup functions', () => {
395+
const options = getDefaultClientOptions({ dsn: PUBLIC_DSN });
396+
client = new ServerRuntimeClient(options);
397+
398+
const cleanups = Array.from({ length: 10 }, () => vi.fn());
399+
400+
cleanups.forEach(cleanup => client.registerCleanup(cleanup));
401+
402+
client.dispose();
403+
404+
cleanups.forEach(cleanup => {
405+
expect(cleanup).toHaveBeenCalledTimes(1);
406+
});
407+
});
323408
});
324409
});

0 commit comments

Comments
 (0)