Skip to content

Commit 9d3ae61

Browse files
JPeer264JPeer264
authored
feat(core,cloudflare): Add dispose to the client for proper cleanup (#19506)
closes #19475 closes [JS-1785](https://linear.app/getsentry/issue/JS-1785/investigate-memory-leaks-in-cloudflare) This is a way to dispose the client entirely. Every request in Cloudflare Workers create their own client. Once the request is done the client would stay in memory forever, unless we `dispose` it after every request. We also have to wait until all `waitUntil`s are finished, otherwise we would loose these traces. The `dispose()` method got added on purpose into the core client, as the `getCurrentClient()` would return a `Client`. The `dispose()` method actually only has functionality inside the `ServerRuntimeClient`, as only the server would need this functionality. There is still a leak in one of the default integrations, but when running load tests against [the reproduction repo](https://github.com/JPeer264/temp-cloudflare-leak) and setting `defaultIntegrations: false`, then no leak is happening. FWIW there will be a separate PR for adding a MemoryProfiler as seen in #19364, to prevent this memory leak in the future.
1 parent 88078a0 commit 9d3ae61

11 files changed

Lines changed: 624 additions & 28 deletions

File tree

‎packages/cloudflare/src/client.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ export class CloudflareClient extends ServerRuntimeClient {
1616
private _spanCompletionPromise: Promise<void> | null = null;
1717
private _resolveSpanCompletion: (() => void) | null = null;
1818

19+
private _unsubscribeSpanStart: (() => void) | null = null;
20+
private _unsubscribeSpanEnd: (() => void) | null = null;
21+
1922
/**
2023
* Creates a new Cloudflare SDK instance.
2124
* @param options Configuration options for this SDK.
@@ -37,7 +40,7 @@ export class CloudflareClient extends ServerRuntimeClient {
3740
this._flushLock = flushLock;
3841

3942
// Track span lifecycle to know when to flush
40-
this.on('spanStart', span => {
43+
this._unsubscribeSpanStart = this.on('spanStart', span => {
4144
const spanId = span.spanContext().spanId;
4245
DEBUG_BUILD && debug.log('[CloudflareClient] Span started:', spanId);
4346
this._pendingSpans.add(spanId);
@@ -49,7 +52,7 @@ export class CloudflareClient extends ServerRuntimeClient {
4952
}
5053
});
5154

52-
this.on('spanEnd', span => {
55+
this._unsubscribeSpanEnd = this.on('spanEnd', span => {
5356
const spanId = span.spanContext().spanId;
5457
DEBUG_BUILD && debug.log('[CloudflareClient] Span ended:', spanId);
5558
this._pendingSpans.delete(spanId);
@@ -99,6 +102,33 @@ export class CloudflareClient extends ServerRuntimeClient {
99102
return super.flush(timeout);
100103
}
101104

105+
/**
106+
* Disposes of the client and releases all resources.
107+
*
108+
* This method clears all Cloudflare-specific state in addition to the base client cleanup.
109+
* It unsubscribes from span lifecycle events and clears pending span tracking.
110+
*
111+
* Call this method after flushing to allow the client to be garbage collected.
112+
* After calling dispose(), the client should not be used anymore.
113+
*/
114+
public override dispose(): void {
115+
DEBUG_BUILD && debug.log('[CloudflareClient] Disposing client...');
116+
117+
super.dispose();
118+
119+
if (this._unsubscribeSpanStart) {
120+
this._unsubscribeSpanStart();
121+
this._unsubscribeSpanStart = null;
122+
}
123+
if (this._unsubscribeSpanEnd) {
124+
this._unsubscribeSpanEnd();
125+
this._unsubscribeSpanEnd = null;
126+
}
127+
128+
this._resetSpanCompletionPromise();
129+
(this as unknown as { _flushLock: ReturnType<typeof makeFlushLock> | void })._flushLock = undefined;
130+
}
131+
102132
/**
103133
* Resets the span completion promise and resolve function.
104134
*/

‎packages/cloudflare/src/flush.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import type { ExecutionContext } from '@cloudflare/workers-types';
2+
import type { Client } from '@sentry/core';
3+
import { flush } from '@sentry/core';
24

35
type FlushLock = {
46
readonly ready: Promise<void>;
@@ -36,3 +38,16 @@ export function makeFlushLock(context: ExecutionContext): FlushLock {
3638
},
3739
});
3840
}
41+
42+
/**
43+
* Flushes the client and then disposes of it to allow garbage collection.
44+
* This should be called at the end of each request to prevent memory leaks.
45+
*
46+
* @param client - The CloudflareClient instance to flush and dispose
47+
* @param timeout - Timeout in milliseconds for the flush operation
48+
* @returns A promise that resolves when flush and dispose are complete
49+
*/
50+
export async function flushAndDispose(client: Client | undefined, timeout = 2000): Promise<void> {
51+
await flush(timeout);
52+
client?.dispose();
53+
}

‎packages/cloudflare/src/handler.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {
22
captureException,
3-
flush,
43
SEMANTIC_ATTRIBUTE_SENTRY_OP,
54
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
65
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
@@ -9,6 +8,7 @@ import {
98
} from '@sentry/core';
109
import { setAsyncLocalStorageAsyncContextStrategy } from './async';
1110
import type { CloudflareOptions } from './client';
11+
import { flushAndDispose } from './flush';
1212
import { isInstrumented, markAsInstrumented } from './instrument';
1313
import { getHonoIntegration } from './integrations/hono';
1414
import { getFinalOptions } from './options';
@@ -113,7 +113,7 @@ export function withSentry<
113113
captureException(e, { mechanism: { handled: false, type: 'auto.faas.cloudflare.scheduled' } });
114114
throw e;
115115
} finally {
116-
waitUntil(flush(2000));
116+
waitUntil(flushAndDispose(client));
117117
}
118118
},
119119
);
@@ -157,7 +157,7 @@ export function withSentry<
157157
captureException(e, { mechanism: { handled: false, type: 'auto.faas.cloudflare.email' } });
158158
throw e;
159159
} finally {
160-
waitUntil(flush(2000));
160+
waitUntil(flushAndDispose(client));
161161
}
162162
},
163163
);
@@ -209,7 +209,7 @@ export function withSentry<
209209
captureException(e, { mechanism: { handled: false, type: 'auto.faas.cloudflare.queue' } });
210210
throw e;
211211
} finally {
212-
waitUntil(flush(2000));
212+
waitUntil(flushAndDispose(client));
213213
}
214214
},
215215
);
@@ -243,7 +243,7 @@ export function withSentry<
243243
captureException(e, { mechanism: { handled: false, type: 'auto.faas.cloudflare.tail' } });
244244
throw e;
245245
} finally {
246-
waitUntil(flush(2000));
246+
waitUntil(flushAndDispose(client));
247247
}
248248
});
249249
},

‎packages/cloudflare/src/request.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type { ExecutionContext, IncomingRequestCfProperties } from '@cloudflare/
22
import {
33
captureException,
44
continueTrace,
5-
flush,
65
getClient,
76
getHttpSpanDetailsFromUrlObject,
87
httpHeadersToSpanAttributes,
@@ -14,6 +13,7 @@ import {
1413
withIsolationScope,
1514
} from '@sentry/core';
1615
import type { CloudflareOptions } from './client';
16+
import { flushAndDispose } from './flush';
1717
import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils';
1818
import { init } from './sdk';
1919
import { classifyResponseStreaming } from './utils/streaming';
@@ -95,7 +95,7 @@ export function wrapRequestHandler(
9595
}
9696
throw e;
9797
} finally {
98-
waitUntil?.(flush(2000));
98+
waitUntil?.(flushAndDispose(client));
9999
}
100100
}
101101

@@ -122,7 +122,7 @@ export function wrapRequestHandler(
122122
if (captureErrors) {
123123
captureException(e, { mechanism: { handled: false, type: 'auto.http.cloudflare' } });
124124
}
125-
waitUntil?.(flush(2000));
125+
waitUntil?.(flushAndDispose(client));
126126
throw e;
127127
}
128128

@@ -149,7 +149,7 @@ export function wrapRequestHandler(
149149
} finally {
150150
reader.releaseLock();
151151
span.end();
152-
waitUntil?.(flush(2000));
152+
waitUntil?.(flushAndDispose(client));
153153
}
154154
})();
155155

@@ -165,14 +165,22 @@ export function wrapRequestHandler(
165165
} catch (e) {
166166
// tee() failed (e.g stream already locked) - fall back to non-streaming handling
167167
span.end();
168-
waitUntil?.(flush(2000));
168+
waitUntil?.(flushAndDispose(client));
169169
return res;
170170
}
171171
}
172172

173173
// Non-streaming response - end span immediately and return original
174174
span.end();
175-
waitUntil?.(flush(2000));
175+
176+
// Don't dispose for protocol upgrades (101 Switching Protocols) - the connection stays alive.
177+
// This includes WebSocket upgrades where webSocketMessage/webSocketClose handlers
178+
// will still be called and may need the client to capture errors.
179+
if (res.status === 101) {
180+
waitUntil?.(client?.flush(2000));
181+
} else {
182+
waitUntil?.(flushAndDispose(client));
183+
}
176184
return res;
177185
});
178186
},

‎packages/cloudflare/src/workflows.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {
2020
} from 'cloudflare:workers';
2121
import { setAsyncLocalStorageAsyncContextStrategy } from './async';
2222
import type { CloudflareOptions } from './client';
23+
import { flushAndDispose } from './flush';
2324
import { addCloudResourceContext } from './scope-utils';
2425
import { init } from './sdk';
2526
import { instrumentContext } from './utils/instrumentContext';
@@ -186,7 +187,7 @@ export function instrumentWorkflowWithSentry<
186187
new WrappedWorkflowStep(event.instanceId, context, options, step),
187188
);
188189
} finally {
189-
context.waitUntil(flush(2000));
190+
context.waitUntil(flushAndDispose(client));
190191
}
191192
});
192193
});

‎packages/cloudflare/src/wrapMethodWithSentry.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { DurableObjectStorage } from '@cloudflare/workers-types';
22
import {
33
captureException,
4-
flush,
54
getClient,
65
isThenable,
76
type Scope,
@@ -12,6 +11,7 @@ import {
1211
withScope,
1312
} from '@sentry/core';
1413
import type { CloudflareOptions } from './client';
14+
import { flushAndDispose } from './flush';
1515
import { isInstrumented, markAsInstrumented } from './instrument';
1616
import { init } from './sdk';
1717

@@ -74,6 +74,8 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
7474
scope.setClient(client);
7575
}
7676

77+
const clientToDispose = currentClient || scope.getClient();
78+
7779
if (!wrapperOptions.spanName) {
7880
try {
7981
if (callback) {
@@ -84,7 +86,7 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
8486
if (isThenable(result)) {
8587
return result.then(
8688
(res: unknown) => {
87-
waitUntil?.(flush(2000));
89+
waitUntil?.(flushAndDispose(clientToDispose));
8890
return res;
8991
},
9092
(e: unknown) => {
@@ -94,12 +96,12 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
9496
handled: false,
9597
},
9698
});
97-
waitUntil?.(flush(2000));
99+
waitUntil?.(flushAndDispose(clientToDispose));
98100
throw e;
99101
},
100102
);
101103
} else {
102-
waitUntil?.(flush(2000));
104+
waitUntil?.(flushAndDispose(clientToDispose));
103105
return result;
104106
}
105107
} catch (e) {
@@ -109,7 +111,7 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
109111
handled: false,
110112
},
111113
});
112-
waitUntil?.(flush(2000));
114+
waitUntil?.(flushAndDispose(clientToDispose));
113115
throw e;
114116
}
115117
}
@@ -128,7 +130,7 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
128130
if (isThenable(result)) {
129131
return result.then(
130132
(res: unknown) => {
131-
waitUntil?.(flush(2000));
133+
waitUntil?.(flushAndDispose(clientToDispose));
132134
return res;
133135
},
134136
(e: unknown) => {
@@ -138,12 +140,12 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
138140
handled: false,
139141
},
140142
});
141-
waitUntil?.(flush(2000));
143+
waitUntil?.(flushAndDispose(clientToDispose));
142144
throw e;
143145
},
144146
);
145147
} else {
146-
waitUntil?.(flush(2000));
148+
waitUntil?.(flushAndDispose(clientToDispose));
147149
return result;
148150
}
149151
} catch (e) {
@@ -153,7 +155,7 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
153155
handled: false,
154156
},
155157
});
156-
waitUntil?.(flush(2000));
158+
waitUntil?.(flushAndDispose(clientToDispose));
157159
throw e;
158160
}
159161
});

0 commit comments

Comments
 (0)