Skip to content

Commit aabf3d8

Browse files
committed
Revert "feat(core): clear up integrations on dispose (#20407)"
This reverts commit 786fdf7.
1 parent 7c38092 commit aabf3d8

11 files changed

Lines changed: 16 additions & 250 deletions

File tree

.size-limit.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ module.exports = [
180180
path: 'packages/vue/build/esm/index.js',
181181
import: createImport('init'),
182182
gzip: true,
183-
limit: '32 KB',
183+
limit: '31 KB',
184184
disablePlugins: ['@size-limit/esbuild'],
185185
},
186186
{
@@ -294,7 +294,7 @@ module.exports = [
294294
path: createCDNPath('bundle.tracing.logs.metrics.min.js'),
295295
gzip: false,
296296
brotli: false,
297-
limit: '144 KB',
297+
limit: '143 KB',
298298
disablePlugins: ['@size-limit/esbuild'],
299299
},
300300
{

packages/core/src/client.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,25 +1169,10 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
11691169
return {};
11701170
}
11711171

1172-
/**
1173-
* Register a cleanup function to be called when the client is disposed.
1174-
* This is useful for integrations that need to clean up global state.
1175-
*
1176-
* NOTE: This is a no-op in the base `Client` class. Subclasses like `ServerRuntimeClient`
1177-
* override this method to actually register and execute cleanup callbacks.
1178-
*/
1179-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
1180-
public registerCleanup(callback: () => void): void {
1181-
// No-op in base class - subclasses override to implement cleanup registration
1182-
}
1183-
11841172
/**
11851173
* Disposes of the client and releases all resources.
11861174
*
1187-
* Subclasses should override this method to clean up their own resources, including invoking
1188-
* any callbacks registered via {@link Client.registerCleanup}. The base implementation is a
1189-
* no-op and does NOT execute registered cleanup callbacks.
1190-
*
1175+
* Subclasses should override this method to clean up their own resources.
11911176
* After calling dispose(), the client should not be used anymore.
11921177
*/
11931178
public dispose(): void {

packages/core/src/instrument/console.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,14 @@ 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.
1211
*
1312
* Use at your own risk, this might break without changelog notice, only used internally.
1413
* @hidden
1514
*/
16-
export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): () => void {
15+
export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): void {
1716
const type = 'console';
18-
const removeHandler = addHandler(type, handler);
17+
addHandler(type, handler);
1918
maybeInstrument(type, instrumentConsole);
20-
return removeHandler;
2119
}
2220

2321
function instrumentConsole(): void {

packages/core/src/instrument/fetch.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,35 +15,31 @@ 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.
1918
*
2019
* Use at your own risk, this might break without changelog notice, only used internally.
2120
* @hidden
2221
*/
2322
export function addFetchInstrumentationHandler(
2423
handler: (data: HandlerDataFetch) => void,
2524
skipNativeFetchCheck?: boolean,
26-
): () => void {
25+
): void {
2726
const type = 'fetch';
28-
const removeHandler = addHandler(type, handler);
27+
addHandler(type, handler);
2928
maybeInstrument(type, () => instrumentFetch(undefined, skipNativeFetchCheck));
30-
return removeHandler;
3129
}
3230

3331
/**
3432
* Add an instrumentation handler for long-lived fetch requests, like consuming server-sent events (SSE) via fetch.
3533
* The handler will resolve the request body and emit the actual `endTimestamp`, so that the
3634
* span can be updated accordingly.
37-
* Returns a function to remove the handler.
3835
*
3936
* Only used internally
4037
* @hidden
4138
*/
42-
export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFetch) => void): () => void {
39+
export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFetch) => void): void {
4340
const type = 'fetch-body-resolved';
44-
const removeHandler = addHandler(type, handler);
41+
addHandler(type, handler);
4542
maybeInstrument(type, () => instrumentFetch(streamHandler));
46-
return removeHandler;
4743
}
4844

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

packages/core/src/instrument/handlers.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,10 @@ 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. Returns a function to remove the handler. */
22-
export function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallback): () => void {
21+
/** Add a handler function. */
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-
};
3525
}
3626

3727
/**

packages/core/src/integrations/console.ts

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

4949
addConsoleBreadcrumb(level, args);
5050
});
51-
52-
client.registerCleanup(unsubscribe);
5351
},
5452
};
5553
});

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

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

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

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportO
3131
export class ServerRuntimeClient<
3232
O extends ClientOptions & ServerRuntimeClientOptions = ServerRuntimeClientOptions,
3333
> extends Client<O> {
34-
private _disposeCallbacks: (() => void)[] = [];
35-
3634
/**
3735
* Creates a new Edge SDK instance.
3836
* @param options Configuration options for this SDK.
@@ -156,13 +154,6 @@ export class ServerRuntimeClient<
156154
return id;
157155
}
158156

159-
/**
160-
* @inheritDoc
161-
*/
162-
public override registerCleanup(callback: () => void): void {
163-
this._disposeCallbacks.push(callback);
164-
}
165-
166157
/**
167158
* Disposes of the client and releases all resources.
168159
*
@@ -177,16 +168,6 @@ export class ServerRuntimeClient<
177168
public override dispose(): void {
178169
DEBUG_BUILD && debug.log('Disposing client...');
179170

180-
// Run all registered cleanup callbacks
181-
for (const callback of this._disposeCallbacks) {
182-
try {
183-
callback();
184-
} catch {
185-
// Ignore errors in cleanup callbacks
186-
}
187-
}
188-
this._disposeCallbacks.length = 0;
189-
190171
for (const hookName of Object.keys(this._hooks)) {
191172
this._hooks[hookName]?.clear();
192173
}
Lines changed: 2 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
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-
});
1+
import { describe, test } from 'vitest';
2+
import { maybeInstrument } from '../../../src/instrument/handlers';
123

134
describe('maybeInstrument', () => {
145
test('does not throw when instrumenting fails', () => {
@@ -21,89 +12,3 @@ describe('maybeInstrument', () => {
2112
maybeInstrument('xhr', undefined as any);
2213
});
2314
});
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: 0 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -320,90 +320,5 @@ 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-
});
408323
});
409324
});

0 commit comments

Comments
 (0)