From d43b738a76ec3deb140006b93528213480e34f5a Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Tue, 17 Feb 2026 08:33:07 -0100 Subject: [PATCH 1/3] feat(core,cloudflare): Add dispose method to Client and CloudflareClient for proper cleanup --- .../cloudflare-workers/package.json | 3 +- .../cloudflare-workers/playwright.config.ts | 4 +- .../cloudflare-workers/tests/memory.test.ts | 50 +++ dev-packages/test-utils/package.json | 4 +- dev-packages/test-utils/src/cdp-client.ts | 272 +++++++++++++++ dev-packages/test-utils/src/index.ts | 6 + .../test-utils/src/memory-profiler.ts | 197 +++++++++++ packages/cloudflare/src/client.ts | 41 ++- packages/cloudflare/src/flush.ts | 15 + packages/cloudflare/src/request.ts | 25 +- packages/cloudflare/test/client.test.ts | 312 ++++++++++++++++++ packages/cloudflare/test/request.test.ts | 189 +++++++++++ packages/core/src/client.ts | 6 +- packages/core/src/server-runtime-client.ts | 29 +- 14 files changed, 1137 insertions(+), 16 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/memory.test.ts create mode 100644 dev-packages/test-utils/src/cdp-client.ts create mode 100644 dev-packages/test-utils/src/memory-profiler.ts create mode 100644 packages/cloudflare/test/client.test.ts diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers/package.json b/dev-packages/e2e-tests/test-applications/cloudflare-workers/package.json index 344337612165..01f6ca3cfe58 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers/package.json +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers/package.json @@ -24,8 +24,7 @@ "@sentry-internal/test-utils": "link:../../../test-utils", "typescript": "^5.5.2", "vitest": "~3.2.0", - "wrangler": "^4.61.0", - "ws": "^8.18.3" + "wrangler": "^4.61.0" }, "volta": { "extends": "../../package.json" diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers/playwright.config.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers/playwright.config.ts index 73abbd951b90..9392f3d57ca3 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers/playwright.config.ts @@ -6,10 +6,12 @@ if (!testEnv) { } const APP_PORT = 38787; +const INSPECTOR_PORT = 9230; const config = getPlaywrightConfig( { - startCommand: `pnpm dev --port ${APP_PORT}`, + // Enable inspector port for memory profiling tests via CDP + startCommand: `pnpm dev --port ${APP_PORT} --inspector-port ${INSPECTOR_PORT}`, port: APP_PORT, }, { diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/memory.test.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/memory.test.ts new file mode 100644 index 000000000000..e6b921e0269a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/memory.test.ts @@ -0,0 +1,50 @@ +import { MemoryProfiler } from '@sentry-internal/test-utils'; +import { expect, test } from '@playwright/test'; + +/** + * Memory leak tests for Cloudflare Workers SDK. + * + * These tests verify that the CloudflareClient.dispose() mechanism properly + * cleans up resources to prevent memory leaks. + * + * The test connects directly to the wrangler dev server's V8 inspector via CDP + * (Chrome DevTools Protocol) on ws://127.0.0.1:9230/ws to take heap snapshots + * of the actual worker isolate. + * + * @see https://developers.cloudflare.com/workers/observability/dev-tools/memory-usage/ + */ + +// Wrangler dev exposes inspector on this port (configured in playwright.config.ts) +const INSPECTOR_PORT = 9230; + +/** + * CDP-based heap snapshot test for Cloudflare Workers. + * + * This test connects directly to the wrangler dev inspector at ws://127.0.0.1:9230/ws + * to profile the actual worker's V8 isolate memory, not a browser. + * + * The wrangler dev server must be running with --inspector-port 9230. + * This is configured in playwright.config.ts. + */ +test.describe('Worker V8 isolate memory tests', () => { + test('worker memory is reclaimed after GC', async ({ baseURL }) => { + const profiler = new MemoryProfiler({ port: INSPECTOR_PORT, debug: true }); + + await profiler.connect(); + await profiler.startProfiling(); + + const numRequests = 50; + + for (let i = 0; i < numRequests; i++) { + const res = await fetch(baseURL!); + expect(res.status).toBe(200); + await res.text(); + } + + const result = await profiler.stopProfiling(); + + expect(result.growthKB).toBeLessThan(800); + + await profiler.close(); + }); +}); diff --git a/dev-packages/test-utils/package.json b/dev-packages/test-utils/package.json index 9bc8e6460786..31f2907d627e 100644 --- a/dev-packages/test-utils/package.json +++ b/dev-packages/test-utils/package.json @@ -44,11 +44,13 @@ "@playwright/test": "~1.56.0" }, "dependencies": { - "express": "^4.21.2" + "express": "^4.21.2", + "ws": "^8.18.0" }, "devDependencies": { "@playwright/test": "~1.56.0", "@sentry/core": "10.39.0", + "@types/ws": "^8.5.10", "eslint-plugin-regexp": "^1.15.0" }, "volta": { diff --git a/dev-packages/test-utils/src/cdp-client.ts b/dev-packages/test-utils/src/cdp-client.ts new file mode 100644 index 000000000000..000c555b1ed5 --- /dev/null +++ b/dev-packages/test-utils/src/cdp-client.ts @@ -0,0 +1,272 @@ +import { WebSocket } from 'ws'; + +/** + * Configuration options for the CDP client. + */ +export interface CDPClientOptions { + /** + * WebSocket URL to connect to (e.g., 'ws://127.0.0.1:9229/ws'). + * Can also use the format 'ws://host:port' without path for standard V8 inspector. + */ + url: string; + + /** + * Number of connection retry attempts before giving up. + * @default 5 + */ + retries?: number; + + /** + * Delay in milliseconds between retry attempts. + * @default 1000 + */ + retryDelayMs?: number; + + /** + * Connection timeout in milliseconds. + * @default 10000 + */ + connectionTimeoutMs?: number; + + /** + * Default timeout for CDP method calls in milliseconds. + * @default 30000 + */ + defaultTimeoutMs?: number; + + /** + * Whether to log debug messages. + * @default false + */ + debug?: boolean; +} + +/** + * Response type for CDP heap usage queries. + */ +export interface HeapUsage { + usedSize: number; + totalSize: number; +} + +interface CDPResponse { + id?: number; + method?: string; + error?: { message: string }; + result?: unknown; +} + +interface PendingRequest { + resolve: (value: unknown) => void; + reject: (error: Error) => void; +} + +/** + * Low-level CDP client for connecting to V8 inspector endpoints. + * + * For memory profiling, prefer using `MemoryProfiler` which provides a higher-level API. + * + * @example + * ```typescript + * const cdp = new CDPClient({ url: 'ws://127.0.0.1:9229/ws' }); + * await cdp.connect(); + * await cdp.send('Runtime.enable'); + * await cdp.close(); + * ``` + */ +export class CDPClient { + private _ws: WebSocket | null; + private _messageId: number; + private _pendingRequests: Map; + private _connected: boolean; + private readonly _options: Required; + + public constructor(options: CDPClientOptions) { + this._ws = null; + this._messageId = 0; + this._pendingRequests = new Map(); + this._connected = false; + this._options = { + retries: 5, + retryDelayMs: 1000, + connectionTimeoutMs: 10000, + defaultTimeoutMs: 30000, + debug: false, + ...options, + }; + } + + /** + * Connect to the V8 inspector WebSocket endpoint. + * Will retry according to the configured retry settings. + */ + public async connect(): Promise { + const { retries, retryDelayMs } = this._options; + + for (let attempt = 1; attempt <= retries; attempt++) { + try { + await this._tryConnect(); + return; + } catch (err) { + this._log(`Connection attempt ${attempt}/${retries} failed:`, (err as Error).message); + if (attempt < retries) { + await new Promise(resolve => setTimeout(resolve, retryDelayMs)); + } else { + throw err; + } + } + } + } + + /** + * Send a CDP method call and wait for the response. + * + * @param method - The CDP method name (e.g., 'HeapProfiler.enable') + * @param params - Optional parameters for the method + * @param timeoutMs - Timeout in milliseconds (defaults to configured defaultTimeoutMs) + * @returns The result from the CDP method + */ + public async send(method: string, params?: Record, timeoutMs?: number): Promise { + if (!this._ws || this._ws.readyState !== WebSocket.OPEN) { + throw new Error('WebSocket not connected'); + } + + const timeout = timeoutMs ?? this._options.defaultTimeoutMs; + const id = ++this._messageId; + const message = JSON.stringify({ id, method, params }); + + this._log('Sending:', method, params || ''); + + return new Promise((resolve, reject) => { + this._pendingRequests.set(id, { + resolve: value => resolve(value as T), + reject, + }); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this._ws!.send(message); + + setTimeout(() => { + if (this._pendingRequests.has(id)) { + this._pendingRequests.delete(id); + reject(new Error(`CDP request ${method} timed out after ${timeout}ms`)); + } + }, timeout); + }); + } + + /** + * Send a CDP method call without waiting for a response. + * Useful for commands that may not return responses in certain V8 environments. + * + * @param method - The CDP method name + * @param params - Optional parameters for the method + * @param settleDelayMs - Time to wait after sending (default: 100ms) + */ + public async sendFireAndForget(method: string, params?: Record, settleDelayMs = 100): Promise { + if (!this._ws || this._ws.readyState !== WebSocket.OPEN) { + throw new Error('WebSocket not connected'); + } + + const id = ++this._messageId; + const message = JSON.stringify({ id, method, params }); + + this._log('Sending (fire-and-forget):', method, params || ''); + + this._ws.send(message); + + // Give the command time to execute + await new Promise(resolve => setTimeout(resolve, settleDelayMs)); + } + + /** + * Check if the client is currently connected. + */ + public isConnected(): boolean { + return this._connected && this._ws?.readyState === WebSocket.OPEN; + } + + /** + * Close the WebSocket connection. + */ + public async close(): Promise { + if (this._ws) { + this._ws.close(); + this._ws = null; + this._connected = false; + } + } + + private _log(...args: unknown[]): void { + if (this._options.debug) { + // eslint-disable-next-line no-console + console.log('[CDPClient]', ...args); + } + } + + private async _tryConnect(): Promise { + const { url, connectionTimeoutMs } = this._options; + + return new Promise((resolve, reject) => { + const timeoutId = setTimeout(() => { + reject(new Error(`Connection to ${url} timed out after ${connectionTimeoutMs}ms`)); + }, connectionTimeoutMs); + + this._ws = new WebSocket(url); + + this._ws.on('open', () => { + clearTimeout(timeoutId); + this._connected = true; + this._log('WebSocket connected to', url); + resolve(); + }); + + this._ws.on('error', (err: Error) => { + clearTimeout(timeoutId); + reject(new Error(`Failed to connect to inspector at ${url}: ${err.message}`)); + }); + + this._ws.on('close', () => { + this._connected = false; + }); + + this._ws.on('message', (data: Buffer) => { + try { + const rawMessage = data.toString(); + this._log('Received raw message:', rawMessage.slice(0, 500)); + + const message = JSON.parse(rawMessage) as CDPResponse; + + // CDP event (not a response to our request) + if (message.method) { + this._log('CDP event:', message.method); + return; + } + + if (message.id !== undefined) { + this._log( + 'CDP response for id:', + message.id, + 'error:', + message.error, + 'has result:', + message.result !== undefined, + ); + const pending = this._pendingRequests.get(message.id); + if (pending) { + this._pendingRequests.delete(message.id); + if (message.error) { + pending.reject(new Error(`CDP error: ${message.error.message}`)); + } else { + pending.resolve(message.result); + } + } else { + this._log('No pending request found for id:', message.id); + } + } + } catch (e) { + this._log('Failed to parse CDP message:', e); + } + }); + }); + } +} diff --git a/dev-packages/test-utils/src/index.ts b/dev-packages/test-utils/src/index.ts index 4a3dfcfaa4c8..5aea8675812e 100644 --- a/dev-packages/test-utils/src/index.ts +++ b/dev-packages/test-utils/src/index.ts @@ -12,3 +12,9 @@ export { export { getPlaywrightConfig } from './playwright-config'; export { createBasicSentryServer, createTestServer } from './server'; + +export { CDPClient } from './cdp-client'; +export type { CDPClientOptions, HeapUsage } from './cdp-client'; + +export { MemoryProfiler } from './memory-profiler'; +export type { MemoryProfilerOptions, MemoryProfilingResult } from './memory-profiler'; diff --git a/dev-packages/test-utils/src/memory-profiler.ts b/dev-packages/test-utils/src/memory-profiler.ts new file mode 100644 index 000000000000..55857b30d0d5 --- /dev/null +++ b/dev-packages/test-utils/src/memory-profiler.ts @@ -0,0 +1,197 @@ +import type { HeapUsage } from './cdp-client'; +import { CDPClient } from './cdp-client'; + +/** + * Options for creating a MemoryProfiler. + */ +export interface MemoryProfilerOptions { + /** + * Inspector port number. + * @default 9229 + */ + port?: number; + + /** + * WebSocket path (e.g., '/ws' for wrangler, '' for Node.js inspector). + * @default '/ws' + */ + path?: string; + + /** + * Host address. + * @default '127.0.0.1' + */ + host?: string; + + /** + * Number of connection retry attempts. + * @default 10 + */ + retries?: number; + + /** + * Delay between retry attempts in milliseconds. + * @default 2000 + */ + retryDelayMs?: number; + + /** + * Delay after garbage collection in milliseconds. + * This gives V8 time to complete GC before measuring. + * @default 500 + */ + gcSettleDelayMs?: number; + + /** + * Enable debug logging. + * @default false + */ + debug?: boolean; +} + +/** + * Result from a memory profiling session. + */ +export interface MemoryProfilingResult { + /** Heap usage at the start of profiling (after GC). */ + baseline: HeapUsage; + /** Heap usage at the end of profiling (after GC). */ + final: HeapUsage; + /** Memory growth in bytes. */ + growthBytes: number; + /** Memory growth in kilobytes. */ + growthKB: number; +} + +/** + * High-level memory profiler for V8 inspector endpoints. + * + * Provides a simple API for memory testing via CDP (Chrome DevTools Protocol). + * Works with any V8 inspector endpoint including: + * - Wrangler dev server (Cloudflare Workers) + * - Node.js inspector (--inspect flag) + * + * @example + * ```typescript + * const profiler = new MemoryProfiler({ port: 9229 }); + * await profiler.connect(); + * + * await profiler.startProfiling(); + * + * // ... run some operations that might leak memory ... + * + * const result = await profiler.stopProfiling(); + * console.log(`Memory growth: ${result.growthKB} KB`); + * + * await profiler.close(); + * ``` + */ +export class MemoryProfiler { + private readonly _cdp: CDPClient; + private readonly _gcSettleDelayMs: number; + private _initialized: boolean; + private _baseline: HeapUsage | null; + + public constructor(options: MemoryProfilerOptions = {}) { + const { + port = 9229, + path = '/ws', + host = '127.0.0.1', + retries = 10, + retryDelayMs = 2000, + gcSettleDelayMs = 500, + debug = false, + } = options; + + this._cdp = new CDPClient({ + url: `ws://${host}:${port}${path}`, + retries, + retryDelayMs, + debug, + }); + this._gcSettleDelayMs = gcSettleDelayMs; + this._initialized = false; + this._baseline = null; + } + + /** + * Connect to the V8 inspector and enable required CDP domains. + */ + public async connect(): Promise { + await this._cdp.connect(); + await this._cdp.send('HeapProfiler.enable'); + await this._cdp.send('Runtime.enable'); + this._initialized = true; + } + + /** + * Check if the profiler is connected to the inspector. + */ + public isConnected(): boolean { + return this._cdp.isConnected() && this._initialized; + } + + /** + * Start a memory profiling session. + * Forces garbage collection and captures baseline heap usage. + */ + public async startProfiling(): Promise { + this._ensureConnected(); + await this._collectGarbage(); + this._baseline = await this._getHeapUsage(); + } + + /** + * Stop the memory profiling session and get the results. + * Forces garbage collection and compares final heap to baseline. + * + * @returns Profiling result with baseline, final heap usage, and growth metrics. + */ + public async stopProfiling(): Promise { + this._ensureConnected(); + if (!this._baseline) { + throw new Error('Profiling not started. Call startProfiling() first.'); + } + + await this._collectGarbage(); + const final = await this._getHeapUsage(); + const baseline = this._baseline; + + // Reset for next profiling session + this._baseline = null; + + const growthBytes = final.usedSize - baseline.usedSize; + const growthKB = growthBytes / 1024; + + return { + baseline, + final, + growthBytes, + growthKB, + }; + } + + /** + * Close the connection to the inspector. + */ + public async close(): Promise { + await this._cdp.close(); + this._initialized = false; + this._baseline = null; + } + + private _ensureConnected(): void { + if (!this._initialized) { + throw new Error('MemoryProfiler not connected. Call connect() first.'); + } + } + + private async _collectGarbage(): Promise { + // Use fire-and-forget because some V8 inspectors (like wrangler) don't respond to this command + await this._cdp.sendFireAndForget('HeapProfiler.collectGarbage', undefined, this._gcSettleDelayMs); + } + + private async _getHeapUsage(): Promise { + return this._cdp.send('Runtime.getHeapUsage'); + } +} diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 3332f71dab90..f6e047f3b721 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -16,6 +16,10 @@ export class CloudflareClient extends ServerRuntimeClient { private _spanCompletionPromise: Promise | null = null; private _resolveSpanCompletion: (() => void) | null = null; + // Store unsubscribe functions for cleanup + private _unsubscribeSpanStart: (() => void) | null = null; + private _unsubscribeSpanEnd: (() => void) | null = null; + /** * Creates a new Cloudflare SDK instance. * @param options Configuration options for this SDK. @@ -37,7 +41,8 @@ export class CloudflareClient extends ServerRuntimeClient { this._flushLock = flushLock; // Track span lifecycle to know when to flush - this.on('spanStart', span => { + // Store unsubscribe functions for cleanup in dispose() + this._unsubscribeSpanStart = this.on('spanStart', span => { const spanId = span.spanContext().spanId; DEBUG_BUILD && debug.log('[CloudflareClient] Span started:', spanId); this._pendingSpans.add(spanId); @@ -49,7 +54,7 @@ export class CloudflareClient extends ServerRuntimeClient { } }); - this.on('spanEnd', span => { + this._unsubscribeSpanEnd = this.on('spanEnd', span => { const spanId = span.spanContext().spanId; DEBUG_BUILD && debug.log('[CloudflareClient] Span ended:', spanId); this._pendingSpans.delete(spanId); @@ -99,6 +104,38 @@ export class CloudflareClient extends ServerRuntimeClient { return super.flush(timeout); } + /** + * Disposes of the client and releases all resources. + * + * This method clears all Cloudflare-specific state in addition to the base client cleanup. + * It unsubscribes from span lifecycle events and clears pending span tracking. + * + * Call this method after flushing to allow the client to be garbage collected. + * After calling dispose(), the client should not be used anymore. + */ + public override dispose(): void { + DEBUG_BUILD && debug.log('[CloudflareClient] Disposing client...'); + + // Unsubscribe from span lifecycle events to break circular references + if (this._unsubscribeSpanStart) { + this._unsubscribeSpanStart(); + this._unsubscribeSpanStart = null; + } + if (this._unsubscribeSpanEnd) { + this._unsubscribeSpanEnd(); + this._unsubscribeSpanEnd = null; + } + + // Clear pending spans and completion promise + this._resetSpanCompletionPromise(); + + // Clear flushLock reference to break context retention + (this as unknown as { _flushLock: ReturnType | void })._flushLock = undefined; + + // Call base class dispose to clean up common state + super.dispose(); + } + /** * Resets the span completion promise and resolve function. */ diff --git a/packages/cloudflare/src/flush.ts b/packages/cloudflare/src/flush.ts index f38c805d0f8b..e4c879594349 100644 --- a/packages/cloudflare/src/flush.ts +++ b/packages/cloudflare/src/flush.ts @@ -1,4 +1,6 @@ import type { ExecutionContext } from '@cloudflare/workers-types'; +import { flush } from '@sentry/core'; +import type { CloudflareClient } from './client'; type FlushLock = { readonly ready: Promise; @@ -36,3 +38,16 @@ export function makeFlushLock(context: ExecutionContext): FlushLock { }, }); } + +/** + * Flushes the client and then disposes of it to allow garbage collection. + * This should be called at the end of each request to prevent memory leaks. + * + * @param client - The CloudflareClient instance to flush and dispose + * @param timeout - Timeout in milliseconds for the flush operation + * @returns A promise that resolves when flush and dispose are complete + */ +export async function flushAndDispose(client: CloudflareClient | undefined, timeout: number): Promise { + await flush(timeout); + client?.dispose(); +} diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index c404e57d01d8..144d48884a9c 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -2,7 +2,6 @@ import type { ExecutionContext, IncomingRequestCfProperties } from '@cloudflare/ import { captureException, continueTrace, - flush, getClient, getHttpSpanDetailsFromUrlObject, httpHeadersToSpanAttributes, @@ -14,6 +13,7 @@ import { withIsolationScope, } from '@sentry/core'; import type { CloudflareOptions } from './client'; +import { flushAndDispose } from './flush'; import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils'; import { init } from './sdk'; import { classifyResponseStreaming } from './utils/streaming'; @@ -95,7 +95,8 @@ export function wrapRequestHandler( } throw e; } finally { - waitUntil?.(flush(2000)); + // Flush and dispose to allow garbage collection + waitUntil?.(flushAndDispose(client, 2000)); } } @@ -122,7 +123,8 @@ export function wrapRequestHandler( if (captureErrors) { captureException(e, { mechanism: { handled: false, type: 'auto.http.cloudflare' } }); } - waitUntil?.(flush(2000)); + // Flush and dispose to allow garbage collection + waitUntil?.(flushAndDispose(client, 2000)); throw e; } @@ -149,7 +151,8 @@ export function wrapRequestHandler( } finally { reader.releaseLock(); span.end(); - waitUntil?.(flush(2000)); + // Flush and dispose to allow garbage collection + waitUntil?.(flushAndDispose(client, 2000)); } })(); @@ -165,14 +168,24 @@ export function wrapRequestHandler( } catch (e) { // tee() failed (e.g stream already locked) - fall back to non-streaming handling span.end(); - waitUntil?.(flush(2000)); + // Flush and dispose to allow garbage collection + waitUntil?.(flushAndDispose(client, 2000)); return res; } } // Non-streaming response - end span immediately and return original span.end(); - waitUntil?.(flush(2000)); + + // Don't dispose for protocol upgrades (101 Switching Protocols) - the connection stays alive. + // This includes WebSocket upgrades where webSocketMessage/webSocketClose handlers + // will still be called and may need the client to capture errors. + if (res.status === 101) { + waitUntil?.(client?.flush(2000)); + } else { + // Flush and dispose to allow garbage collection + waitUntil?.(flushAndDispose(client, 2000)); + } return res; }); }, diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts new file mode 100644 index 000000000000..ecf85c0f4c2c --- /dev/null +++ b/packages/cloudflare/test/client.test.ts @@ -0,0 +1,312 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { setAsyncLocalStorageAsyncContextStrategy } from '../src/async'; +import { CloudflareClient, type CloudflareClientOptions } from '../src/client'; +import { makeFlushLock } from '../src/flush'; + +const MOCK_CLIENT_OPTIONS: CloudflareClientOptions = { + dsn: 'https://public@dsn.ingest.sentry.io/1337', + stackParser: () => [], + integrations: [], + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: vi.fn().mockResolvedValue(true), + }), +}; + +describe('CloudflareClient', () => { + beforeAll(() => { + setAsyncLocalStorageAsyncContextStrategy(); + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('dispose()', () => { + it('unsubscribes from span lifecycle events', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + // Access the private unsubscribe functions to verify they exist + const privateClient = client as unknown as { + _unsubscribeSpanStart: (() => void) | null; + _unsubscribeSpanEnd: (() => void) | null; + }; + + expect(privateClient._unsubscribeSpanStart).not.toBeNull(); + expect(privateClient._unsubscribeSpanEnd).not.toBeNull(); + + client.dispose(); + + expect(privateClient._unsubscribeSpanStart).toBeNull(); + expect(privateClient._unsubscribeSpanEnd).toBeNull(); + }); + + it('clears pending spans tracking', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _pendingSpans: Set; + _spanCompletionPromise: Promise | null; + _resolveSpanCompletion: (() => void) | null; + }; + + // Add some pending spans + privateClient._pendingSpans.add('span1'); + privateClient._pendingSpans.add('span2'); + privateClient._spanCompletionPromise = new Promise(() => {}); + privateClient._resolveSpanCompletion = () => {}; + + expect(privateClient._pendingSpans.size).toBe(2); + + client.dispose(); + + expect(privateClient._pendingSpans.size).toBe(0); + expect(privateClient._spanCompletionPromise).toBeNull(); + expect(privateClient._resolveSpanCompletion).toBeNull(); + }); + + it('clears flushLock reference', () => { + const mockContext = { + waitUntil: vi.fn(), + passThroughOnException: vi.fn(), + }; + const flushLock = makeFlushLock(mockContext as any); + + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + flushLock, + }); + + const privateClient = client as unknown as { + _flushLock: ReturnType | void; + }; + + expect(privateClient._flushLock).toBeDefined(); + + client.dispose(); + + expect(privateClient._flushLock).toBeUndefined(); + }); + + it('clears hooks', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + // Add a hook + const hookCallback = vi.fn(); + client.on('beforeEnvelope', hookCallback); + + const privateClient = client as unknown as { + _hooks: Record | undefined>; + }; + + // Verify hook was registered - check that there are hooks with actual Sets + const hooksWithSets = Object.values(privateClient._hooks).filter(v => v instanceof Set); + expect(hooksWithSets.length).toBeGreaterThan(0); + + client.dispose(); + + // All hooks should be cleared (set to undefined) + const hooksWithSetsAfter = Object.values(privateClient._hooks).filter(v => v instanceof Set); + expect(hooksWithSetsAfter.length).toBe(0); + }); + + it('clears event processors', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + // Add an event processor + client.addEventProcessor(event => event); + + const privateClient = client as unknown as { + _eventProcessors: unknown[]; + }; + + // SDK adds some default processors, so length should be >= 1 + const initialLength = privateClient._eventProcessors.length; + expect(initialLength).toBeGreaterThan(0); + + client.dispose(); + + expect(privateClient._eventProcessors.length).toBe(0); + }); + + it('clears integrations', () => { + const mockIntegration = { + name: 'MockIntegration', + setupOnce: vi.fn(), + }; + + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + integrations: [mockIntegration], + }); + + // Need to call init() to setup integrations + client.init(); + + const privateClient = client as unknown as { + _integrations: Record; + }; + + // Integration should be registered + expect(privateClient._integrations['MockIntegration']).toBeDefined(); + expect(privateClient._integrations['MockIntegration']).not.toBeUndefined(); + + client.dispose(); + + // Integration reference should be cleared (set to undefined) + expect(privateClient._integrations['MockIntegration']).toBeUndefined(); + }); + + it('clears transport reference', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _transport?: unknown; + }; + + expect(privateClient._transport).toBeDefined(); + + client.dispose(); + + expect(privateClient._transport).toBeUndefined(); + }); + + it('clears outcomes tracking', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _outcomes: Record; + }; + + // Add some outcomes + privateClient._outcomes['reason:error:outcome1'] = 5; + privateClient._outcomes['reason:error:outcome2'] = 10; + + // Verify we have actual values + const validOutcomes = Object.values(privateClient._outcomes).filter(v => v !== undefined); + expect(validOutcomes.length).toBe(2); + + client.dispose(); + + // All outcomes should be set to undefined + const validOutcomesAfter = Object.values(privateClient._outcomes).filter(v => v !== undefined); + expect(validOutcomesAfter.length).toBe(0); + }); + + it('can be called multiple times safely', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + // Should not throw when called multiple times + expect(() => { + client.dispose(); + client.dispose(); + client.dispose(); + }).not.toThrow(); + }); + + it('does not break event emission after spanStart unsubscribe', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + // Dispose which unsubscribes from span events + client.dispose(); + + // Should not throw when emitting span events after dispose + expect(() => { + client.emit('spanStart', {} as any); + client.emit('spanEnd', {} as any); + }).not.toThrow(); + }); + }); + + describe('span lifecycle tracking', () => { + it('tracks pending spans when spanStart is emitted', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _pendingSpans: Set; + _spanCompletionPromise: Promise | null; + }; + + expect(privateClient._pendingSpans.size).toBe(0); + expect(privateClient._spanCompletionPromise).toBeNull(); + + // Emit spanStart + const mockSpan = { + spanContext: () => ({ spanId: 'test-span-id' }), + }; + client.emit('spanStart', mockSpan as any); + + expect(privateClient._pendingSpans.has('test-span-id')).toBe(true); + expect(privateClient._spanCompletionPromise).not.toBeNull(); + }); + + it('removes pending span when spanEnd is emitted', async () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _pendingSpans: Set; + _spanCompletionPromise: Promise | null; + }; + + const mockSpan = { + spanContext: () => ({ spanId: 'test-span-id' }), + }; + + // Start span + client.emit('spanStart', mockSpan as any); + expect(privateClient._pendingSpans.has('test-span-id')).toBe(true); + + // End span + client.emit('spanEnd', mockSpan as any); + expect(privateClient._pendingSpans.has('test-span-id')).toBe(false); + }); + + it('resolves completion promise when all spans end', async () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _pendingSpans: Set; + _spanCompletionPromise: Promise | null; + }; + + const mockSpan1 = { spanContext: () => ({ spanId: 'span-1' }) }; + const mockSpan2 = { spanContext: () => ({ spanId: 'span-2' }) }; + + // Start both spans + client.emit('spanStart', mockSpan1 as any); + client.emit('spanStart', mockSpan2 as any); + + const completionPromise = privateClient._spanCompletionPromise; + expect(completionPromise).not.toBeNull(); + + // End first span - promise should still exist + client.emit('spanEnd', mockSpan1 as any); + expect(privateClient._pendingSpans.size).toBe(1); + + // End second span - promise should be resolved and reset + client.emit('spanEnd', mockSpan2 as any); + expect(privateClient._pendingSpans.size).toBe(0); + + // The original promise should resolve + await expect(completionPromise).resolves.toBeUndefined(); + }); + + it('does not track spans after dispose', () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + client.dispose(); + + const privateClient = client as unknown as { + _pendingSpans: Set; + }; + + const mockSpan = { + spanContext: () => ({ spanId: 'test-span-id' }), + }; + + // Emit spanStart after dispose - should not be tracked + client.emit('spanStart', mockSpan as any); + expect(privateClient._pendingSpans.has('test-span-id')).toBe(false); + }); + }); +}); diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index 94b5d89e4ae0..5160d8976e9b 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -377,3 +377,192 @@ function createMockExecutionContext(): ExecutionContext { passThroughOnException: vi.fn(), }; } + +describe('flushAndDispose', () => { + test('dispose is called after flush completes', async () => { + const context = createMockExecutionContext(); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); + (context as any).waitUntil = waitUntil; + + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true); + + await wrapRequestHandler({ options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, () => { + const response = new Response('test'); + response.headers.set('content-length', '4'); + return response; + }); + + // Wait for all waitUntil promises to resolve + await Promise.all(waits); + + expect(flushSpy).toHaveBeenCalled(); + expect(disposeSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); + + test('dispose is called after handler throws error', async () => { + const context = createMockExecutionContext(); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); + (context as any).waitUntil = waitUntil; + + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true); + + try { + await wrapRequestHandler({ options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, () => { + throw new Error('test error'); + }); + } catch { + // Expected to throw + } + + // Wait for all waitUntil promises to resolve + await Promise.all(waits); + + expect(disposeSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); + + test('dispose is called for OPTIONS requests', async () => { + const context = createMockExecutionContext(); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); + (context as any).waitUntil = waitUntil; + + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true); + + await wrapRequestHandler( + { + options: MOCK_OPTIONS, + request: new Request('https://example.com', { method: 'OPTIONS' }), + context, + }, + () => new Response('', { status: 200 }), + ); + + // Wait for all waitUntil promises to resolve + await Promise.all(waits); + + expect(disposeSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); + + test('dispose is called for HEAD requests', async () => { + const context = createMockExecutionContext(); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); + (context as any).waitUntil = waitUntil; + + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true); + + await wrapRequestHandler( + { + options: MOCK_OPTIONS, + request: new Request('https://example.com', { method: 'HEAD' }), + context, + }, + () => new Response('', { status: 200 }), + ); + + // Wait for all waitUntil promises to resolve + await Promise.all(waits); + + expect(disposeSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); + + test('dispose is called after streaming response completes', async () => { + const context = createMockExecutionContext(); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); + (context as any).waitUntil = waitUntil; + + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true); + + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('chunk1')); + controller.enqueue(new TextEncoder().encode('chunk2')); + controller.close(); + }, + }); + + const result = await wrapRequestHandler( + { options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, + () => new Response(stream), + ); + + // Consume the response to trigger stream completion + await result.text(); + + // Wait for all waitUntil promises to resolve + await Promise.all(waits); + + expect(disposeSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); + + test('dispose is NOT called for protocol upgrade responses (status 101)', async () => { + const context = createMockExecutionContext(); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); + (context as any).waitUntil = waitUntil; + + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + const flushSpy = vi.spyOn(CloudflareClient.prototype, 'flush').mockResolvedValue(true); + + // Create a mock protocol upgrade response (Node.js Response doesn't allow status 101) + // In Cloudflare Workers, this is a valid response for WebSocket upgrades and other protocol switches + const mockWebSocketResponse = { + status: 101, + statusText: 'Switching Protocols', + headers: new Headers(), + body: null, + ok: false, + redirected: false, + type: 'basic' as ResponseType, + url: '', + clone: () => mockWebSocketResponse, + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + blob: () => Promise.resolve(new Blob()), + formData: () => Promise.resolve(new FormData()), + json: () => Promise.resolve({}), + text: () => Promise.resolve(''), + bodyUsed: false, + bytes: () => Promise.resolve(new Uint8Array()), + } as Response; + + await wrapRequestHandler( + { options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, + () => mockWebSocketResponse, + ); + + // Wait for all waitUntil promises to resolve + await Promise.all(waits); + + // dispose should NOT be called for protocol upgrades (101) since the connection stays alive + // and subsequent handlers (e.g., webSocketMessage/webSocketClose) may still need the client + expect(disposeSpy).not.toHaveBeenCalled(); + // But flush should still be called + expect(flushSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); +}); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 588c8a62e97e..072b220175a2 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -203,12 +203,12 @@ export abstract class Client { protected _eventProcessors: EventProcessor[]; /** Holds flushable */ - private _outcomes: { [key: string]: number }; + protected _outcomes: { [key: string]: number }; // eslint-disable-next-line @typescript-eslint/ban-types - private _hooks: Record>; + protected _hooks: Record>; - private _promiseBuffer: PromiseBuffer; + protected _promiseBuffer: PromiseBuffer; /** * Initializes this client instance. diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index d1ae8e9063e6..8ec299937116 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -10,10 +10,11 @@ import type { Event, EventHint } from './types-hoist/event'; import type { ClientOptions } from './types-hoist/options'; import type { ParameterizedString } from './types-hoist/parameterize'; import type { SeverityLevel } from './types-hoist/severity'; -import type { BaseTransportOptions } from './types-hoist/transport'; +import type { BaseTransportOptions, Transport } from './types-hoist/transport'; import { debug } from './utils/debug-logger'; import { eventFromMessage, eventFromUnknownInput } from './utils/eventbuilder'; import { uuid4 } from './utils/misc'; +import type { PromiseBuffer } from './utils/promisebuffer'; import { resolvedSyncPromise } from './utils/syncpromise'; import { _getTraceInfoFromScope } from './utils/trace-info'; @@ -152,6 +153,32 @@ export class ServerRuntimeClient< return id; } + /** + * Disposes of the client and releases all resources. + * + * This method clears all internal state to allow the client to be garbage collected. + * It clears hooks, event processors, integrations, transport, and other internal references. + * + * Call this method after flushing to allow the client to be garbage collected. + * After calling dispose(), the client should not be used anymore. + * + * Subclasses should override this method to clean up their own resources and call `super.dispose()`. + */ + public dispose(): void { + DEBUG_BUILD && debug.log('Disposing client...'); + + for (const hookName of Object.keys(this._hooks)) { + this._hooks[hookName]?.clear(); + } + + this._hooks = {}; + this._eventProcessors.length = 0; + this._integrations = {}; + this._outcomes = {}; + (this as unknown as { _transport?: Transport })._transport = undefined; + (this as unknown as { _promiseBuffer?: PromiseBuffer })._promiseBuffer = undefined; + } + /** * @inheritDoc */ From 437de0d7b0167d948c1fe5b5f1f78341555e9be2 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 18 Feb 2026 07:30:09 -0100 Subject: [PATCH 2/3] fixup! feat(core,cloudflare): Add dispose method to Client and CloudflareClient for proper cleanup --- dev-packages/test-utils/src/memory-profiler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/test-utils/src/memory-profiler.ts b/dev-packages/test-utils/src/memory-profiler.ts index 55857b30d0d5..285e0f9fb373 100644 --- a/dev-packages/test-utils/src/memory-profiler.ts +++ b/dev-packages/test-utils/src/memory-profiler.ts @@ -38,7 +38,7 @@ export interface MemoryProfilerOptions { /** * Delay after garbage collection in milliseconds. * This gives V8 time to complete GC before measuring. - * @default 500 + * @default 2000 */ gcSettleDelayMs?: number; @@ -99,7 +99,7 @@ export class MemoryProfiler { host = '127.0.0.1', retries = 10, retryDelayMs = 2000, - gcSettleDelayMs = 500, + gcSettleDelayMs = 2000, debug = false, } = options; From 899dd7c8711ed27940f9208306701185b5a3f087 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 18 Feb 2026 09:44:56 -0100 Subject: [PATCH 3/3] fixup! fixup! feat(core,cloudflare): Add dispose method to Client and CloudflareClient for proper cleanup --- dev-packages/test-utils/src/memory-profiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/test-utils/src/memory-profiler.ts b/dev-packages/test-utils/src/memory-profiler.ts index 285e0f9fb373..07e146eea663 100644 --- a/dev-packages/test-utils/src/memory-profiler.ts +++ b/dev-packages/test-utils/src/memory-profiler.ts @@ -99,7 +99,7 @@ export class MemoryProfiler { host = '127.0.0.1', retries = 10, retryDelayMs = 2000, - gcSettleDelayMs = 2000, + gcSettleDelayMs = 3000, debug = false, } = options;