From a5ec6bd6b65a4aa243a442f222be33ae8af31e91 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 7 Aug 2026 10:37:03 +0200 Subject: [PATCH 01/14] feat(cloudflare): Add cacheClient to reuse the client across invocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building and disposing a client per invocation costs real time on every request, and in a Durable Object it also loses data: there is no `waitUntil` boundary that dependably extends execution, so anything captured after the handler returned went to a client that had already been disposed. Enabled by default, this caches one client per isolate. The first initialization wins for the isolate's lifetime: a later init with different options reuses that client, and a new deployment always starts fresh isolates, so clients are always built from the current version's options. A cached client is flushed but not disposed at an invocation boundary, and it is re-bound to the current scope on every invocation — otherwise `initialScope` would apply only to an isolate's first invocation, and a client disposed by a competing init would keep being handed out. A cached client whose transport is gone is evicted rather than returned. Because a reused client never reaches an end-of-invocation flush, delivery is eager: the new `afterEnvelope` hook on the core client drains the transport buffer as soon as an envelope has been accepted, and logs and metrics drain on a debounced hook so they are batched rather than sent one at a time. Spans that end after the invocation's flush point are delivered through core's `flushTraceSpans` hook, which flushes only that trace's bucket from the span streaming buffer. The per-invocation flush lock and span tracking are skipped, since binding a client that outlives the invocation to one invocation's lock would make later flushes wait on that invocation's work forever. A shared client also shares integration state, so dedupe works across invocations: the same error raised by two separate requests is reported only once. Uncached behavior is unchanged; pass `cacheClient: false` to restore it. Co-authored-by: Cursor --- .../suites/cache-client/index.ts | 257 +++++++++ .../suites/cache-client/test.ts | 321 +++++++++++ .../suites/cache-client/wrangler.jsonc | 18 + packages/cloudflare/src/baseSdk.ts | 8 +- packages/cloudflare/src/client.ts | 303 +++++++++-- packages/cloudflare/src/clientCache.ts | 23 + packages/cloudflare/src/flush.ts | 14 +- .../worker/instrumentEmail.ts | 3 + .../worker/instrumentQueue.ts | 3 + .../worker/instrumentScheduled.ts | 3 + .../instrumentations/worker/instrumentTail.ts | 3 + packages/cloudflare/src/request.ts | 2 +- packages/cloudflare/src/sdk.ts | 46 +- packages/cloudflare/src/transport.ts | 2 +- .../cloudflare/src/utils/invocationContext.ts | 72 +++ .../cloudflare/src/utils/invocationScope.ts | 11 +- .../cloudflare/src/wrapMethodWithSentry.ts | 5 +- packages/cloudflare/test/client.test.ts | 514 ++++++++++++++++++ packages/cloudflare/test/flush.test.ts | 13 + .../worker/instrumentEmail.test.ts | 4 +- .../worker/instrumentFetch.test.ts | 4 +- .../worker/instrumentQueue.test.ts | 4 +- .../worker/instrumentScheduled.test.ts | 4 +- .../worker/instrumentTail.test.ts | 4 +- packages/cloudflare/test/request.test.ts | 193 +++++++ packages/cloudflare/test/sdk.test.ts | 207 ++++++- packages/cloudflare/test/testUtils.ts | 2 + .../test/utils/invocationContext.test.ts | 76 +++ packages/cloudflare/test/workflow.test.ts | 26 +- 29 files changed, 2089 insertions(+), 56 deletions(-) create mode 100644 dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/cache-client/wrangler.jsonc create mode 100644 packages/cloudflare/src/clientCache.ts create mode 100644 packages/cloudflare/src/utils/invocationContext.ts create mode 100644 packages/cloudflare/test/utils/invocationContext.test.ts diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts new file mode 100644 index 000000000000..1ef5f389fcd3 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts @@ -0,0 +1,257 @@ +import * as Sentry from '@sentry/cloudflare'; +import { DurableObject } from 'cloudflare:workers'; + +interface Env { + SENTRY_DSN: string; + CACHE_DO: DurableObjectNamespace; + NO_CACHE_DO: DurableObjectNamespace; +} + +/** + * Sync KV and SQL work against the DO's own storage, which the SDK instruments into `db` spans. + * Used to check that those spans still reach the transport from inside a Durable Object, where a + * cached client never hits an invocation-boundary flush and has to rely on the eager drain. + */ +function runStorageOps(ctx: DurableObjectState): { listSize: number; rows: number } { + ctx.storage.kv.put('cache-key', { hello: 'sync' }); + ctx.storage.kv.get('cache-key'); + const entries = [...ctx.storage.kv.list()]; + ctx.storage.kv.delete('cache-key'); + + ctx.storage.sql.exec('CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY, name TEXT)'); + ctx.storage.sql.exec('INSERT INTO users (name) VALUES (?)', 'Alice'); + const rows = ctx.storage.sql.exec('SELECT * FROM users').toArray(); + + return { listSize: entries.length, rows: rows.length }; +} + +function startDetachedWork(message: string): string { + void (async () => { + await new Promise(r => setTimeout(r, 3000)); + await Sentry.startSpan({ name: 'do.detached-task', op: 'task' }, async () => { + Sentry.logger.info(`Detached log: ${message}`); + Sentry.metrics.count('do.detached', 1); + Sentry.captureException(new Error(message)); + }); + })(); + return `Detached work started: ${message}`; +} + +// DO with cacheClient: true (the default) — detached work events SHOULD be captured +class CacheDurableObjectBase extends DurableObject { + async echo(n: number): Promise { + return n; + } + + async handlerError(instanceId: string): Promise { + throw new Error(`Cache DO handler error from ${instanceId}`); + } + + async dedupe(): Promise { + Sentry.captureException(new Error('Same error')); + return 'dedupe test'; + } + + async scopeCheck(seed: boolean): Promise { + if (seed) { + Sentry.setTag('seeded_tag', 'from-seeding-call'); + Sentry.setUser({ id: 'user-from-seeding-call' }); + } + Sentry.captureException(new Error(seed ? 'Cache scope seed' : 'Cache scope probe')); + return 'ok'; + } + + async storage(): Promise { + const { listSize, rows } = runStorageOps(this.ctx); + return `cache storage ${listSize}/${rows}`; + } + + async fetch(request: Request): Promise { + const url = new URL(request.url); + if (url.pathname === '/detached') { + return new Response(startDetachedWork(`Detached work from cache DO ${url.searchParams.get('id')}`)); + } + if (url.pathname === '/streaming') { + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('chunk1')); + controller.enqueue(new TextEncoder().encode('chunk2')); + controller.close(); + }, + }); + return new Response(stream, { headers: { 'content-type': 'text/event-stream' } }); + } + return new Response('Cache DO'); + } +} + +// DO with cacheClient: false — detached work events should NOT be captured +class NoCacheDurableObjectBase extends DurableObject { + async handlerError(instanceId: string): Promise { + throw new Error(`No-cache DO handler error from ${instanceId}`); + } + + async dedupe(): Promise { + Sentry.captureException(new Error('Same error')); + return 'dedupe test'; + } + + async storage(): Promise { + const { listSize, rows } = runStorageOps(this.ctx); + return `no-cache storage ${listSize}/${rows}`; + } + + async fetch(request: Request): Promise { + const url = new URL(request.url); + if (url.pathname === '/detached') { + return new Response(startDetachedWork(`Detached work from no-cache DO ${url.searchParams.get('id')}`)); + } + return new Response('No-cache DO'); + } +} + +export const CacheDurableObject = Sentry.instrumentDurableObjectWithSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1, + enableLogs: true, + enableRpcTracePropagation: true, + }), + CacheDurableObjectBase, +); + +export const NoCacheDurableObject = Sentry.instrumentDurableObjectWithSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1, + cacheClient: false, + enableRpcTracePropagation: true, + }), + NoCacheDurableObjectBase, +); + +export default Sentry.withSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1, + enableLogs: true, + enableRpcTracePropagation: true, + }), + { + async fetch(request, env, ctx) { + const url = new URL(request.url); + const instanceId = url.searchParams.get('id') || 'default'; + + // Work that finishes AFTER the response: a post-response span tree plus a + // log, metric and error, all registered via waitUntil. This is the worker-side + // half of the #22545 lifecycle (the DO-side half is /detached). + if (url.pathname === '/post-response') { + ctx.waitUntil( + Sentry.startSpan({ name: 'checkout.post-response', op: 'task' }, async () => { + Sentry.logger.info('checkout post-response log'); + Sentry.metrics.count('checkout.processed', 1); + await new Promise(r => setTimeout(r, 50)); + await Sentry.startSpan({ name: 'checkout.notify-webhook', op: 'http.client' }, async () => { + await new Promise(r => setTimeout(r, 25)); + Sentry.captureException(new Error('Webhook delivery failed')); + }); + }), + ); + return new Response('checkout accepted'); + } + + // Fan a single request out into N sequential DO RPC calls — every RPC span must + // land in this request's trace when RPC trace propagation is on. + if (url.pathname === '/burst') { + const n = Math.min(Number(url.searchParams.get('n')) || 1, 20); + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`burst-${instanceId}`), + ) as DurableObjectStub; + + let sum = 0; + for (let i = 0; i < n; i++) { + sum += (await stub.echo(i)) as number; + } + return Response.json({ calls: n, sum }); + } + + // Cache DO RPC calls + if (url.pathname === '/cache/handler-error') { + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`cache-do-${instanceId}`), + ) as DurableObjectStub; + await stub.handlerError(instanceId); + } + + if (url.pathname === '/cache/dedupe') { + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`cache-do-${instanceId}`), + ) as DurableObjectStub; + const result = await stub.dedupe(); + return new Response(String(result)); + } + + if (url.pathname === '/cache/scope') { + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`cache-do-${instanceId}`), + ) as DurableObjectStub; + return new Response(await stub.scopeCheck(url.searchParams.get('seed') === '1')); + } + + // Cache DO fetch calls — detached work goes through fetch (matching the #22545 repro), + // since the DO fetch handler always initializes the DO's own client + if (url.pathname === '/cache/detached') { + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`cache-do-${instanceId}`), + ) as DurableObjectStub; + return stub.fetch(new Request(`http://do/detached?id=${instanceId}`)); + } + + if (url.pathname === '/cache/storage') { + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`cache-do-${instanceId}`), + ) as DurableObjectStub; + return new Response(await stub.storage()); + } + + if (url.pathname === '/cache/streaming') { + const stub = env.CACHE_DO.get( + env.CACHE_DO.idFromName(`cache-do-${instanceId}`), + ) as DurableObjectStub; + return stub.fetch(new Request('http://do/streaming')); + } + + // No-cache DO calls + if (url.pathname === '/no-cache/handler-error') { + const stub = env.NO_CACHE_DO.get( + env.NO_CACHE_DO.idFromName(`no-cache-do-${instanceId}`), + ) as DurableObjectStub; + await stub.handlerError(instanceId); + } + + if (url.pathname === '/no-cache/dedupe') { + const stub = env.NO_CACHE_DO.get( + env.NO_CACHE_DO.idFromName(`no-cache-do-${instanceId}`), + ) as DurableObjectStub; + const result = await stub.dedupe(); + return new Response(String(result)); + } + + if (url.pathname === '/no-cache/storage') { + const stub = env.NO_CACHE_DO.get( + env.NO_CACHE_DO.idFromName(`no-cache-do-${instanceId}`), + ) as DurableObjectStub; + return new Response(await stub.storage()); + } + + if (url.pathname === '/no-cache/detached') { + const stub = env.NO_CACHE_DO.get( + env.NO_CACHE_DO.idFromName(`no-cache-do-${instanceId}`), + ) as DurableObjectStub; + return stub.fetch(new Request(`http://do/detached?id=${instanceId}`)); + } + + return new Response('Hello World!'); + }, + } satisfies ExportedHandler, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts new file mode 100644 index 000000000000..dde6864391c6 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts @@ -0,0 +1,321 @@ +import type { Envelope, Event } from '@sentry/core'; +import { describe, expect, it } from 'vitest'; +import { createRunner } from '../../runner'; + +type Mechanism = { type: string; handled: boolean }; + +/** + * Matches an error event by exception value and capture mechanism. + * + * Callback-style (instead of exact `eventEnvelope` matching) because Durable Object + * RPC events carry no `request` and their trace context varies with propagation — + * only the exception payload is stable. Non-matching envelopes (worker-side duplicate + * captures, transactions) are dropped by the runner's unordered mode. + */ +function errorEventExpectation(value: string, mechanism: Mechanism) { + return (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event).toEqual( + expect.objectContaining({ + level: 'error', + exception: { + values: [ + expect.objectContaining({ + type: 'Error', + value, + stacktrace: { frames: expect.any(Array) }, + mechanism, + }), + ], + }, + }), + ); + }; +} + +const DO_MECHANISM: Mechanism = { type: 'auto.faas.cloudflare.durable_object', handled: false }; +// Direct `captureException` calls (not routed through a wrapped handler) always get this mechanism +const CAPTURE_MECHANISM: Mechanism = { type: 'generic', handled: true }; + +/** span-v2 streamed envelope payload. */ +type SpanV2Payload = { + items?: Array<{ + name?: string; + trace_id?: string; + attributes?: Record; + }>; +}; + +/** + * Matches a span-v2 envelope containing at least one span with the given name. + * Spans are matched loosely because batching can coalesce multiple spans of one + * trace into a single envelope. + */ +function spanEnvelopeExpectation(spanName: string) { + return (envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as SpanV2Payload; + expect(payload.items?.some(span => span.name === spanName)).toBe(true); + }; +} + +it('cacheClient: false - DO handler error is captured', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect(errorEventExpectation('No-cache DO handler error from instance-1', DO_MECHANISM)) + .expect(errorEventExpectation('No-cache DO handler error from instance-2', DO_MECHANISM)) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/no-cache/handler-error?id=instance-1', { expectError: true }); + await runner.makeRequest('get', '/no-cache/handler-error?id=instance-2', { expectError: true }); + await runner.completed(); +}); + +it('cacheClient: true - DO handler error is captured', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect(errorEventExpectation('Cache DO handler error from instance-1', DO_MECHANISM)) + .expect(errorEventExpectation('Cache DO handler error from instance-2', DO_MECHANISM)) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/cache/handler-error?id=instance-1', { expectError: true }); + await runner.makeRequest('get', '/cache/handler-error?id=instance-2', { expectError: true }); + await runner.completed(); +}); + +it('cacheClient: true - detached work events ARE captured', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect(errorEventExpectation('Detached work from cache DO instance-1', CAPTURE_MECHANISM)) + .expect(errorEventExpectation('Detached work from cache DO instance-2', CAPTURE_MECHANISM)) + // Logs batch client-side and the idle drain timer is disabled for this runtime, so a + // log only ever becomes an envelope if the cached client drains its log buffer on + // capture. Without that, detached logs are silently dropped while errors still arrive. + .expect((envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as { items?: Array<{ body?: string }> }; + expect(payload.items?.some(log => log.body?.startsWith('Detached log: Detached work from cache DO'))).toBe(true); + }) + // The detached span itself: captured in work that starts after the RPC invocation + // settled, so it only survives because the cached client delivers it eagerly. + .expect(spanEnvelopeExpectation('do.detached-task')) + // The detached metric, delivered via the same eager drain as the span and log. + .expect((envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as { items?: Array<{ name?: string }> }; + expect(payload.items?.some(metric => metric.name === 'do.detached')).toBe(true); + }) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/cache/detached?id=instance-1'); + await runner.makeRequest('get', '/cache/detached?id=instance-2'); + await runner.completed(); +}); + +it('cacheClient: false - repro #22545: detached work events are silently dropped', async ({ signal }) => { + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + // Make the request that spawns detached work + await runner.makeRequest('get', '/no-cache/detached?id=repro-1'); + + // With cacheClient: false, the client is disposed after the handler returns, + // so the detached work's captureException (3s later) is silently dropped. + // We verify by waiting for the event with a timeout — if it doesn't arrive, + // the event was silently dropped as expected. + const result = await Promise.race([ + runner.makeRequestAndWaitForEnvelope('get', '/no-cache/detached?id=repro-2', () => { + throw new Error('Received an event that should have been dropped with cacheClient: false'); + }), + // Timeout: resolve with 'timeout' if no event arrives within 5s + new Promise(resolve => setTimeout(() => resolve('timeout'), 5000)), + ]); + + // The event should NOT have been received (timeout should win the race) + expect(result).toBe('timeout'); +}); + +it('cacheClient: true - dedupe drops the same error across invocations', async ({ signal }) => { + // A shared client shares its dedupe state, so the same error captured by two separate + // invocations is reported only once — the second is dropped as a duplicate. + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + await runner.makeRequestAndWaitForEnvelope( + 'get', + '/cache/dedupe?id=dedupe-shared', + errorEventExpectation('Same error', CAPTURE_MECHANISM), + ); + + // Second and third invocations capture the same error, but dedupe drops them. + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); +}); + +it('cacheClient: false - dedupe does not persist across invocations', async ({ signal }) => { + // A fresh client per invocation means fresh dedupe state, so each invocation reports + // the same error independently. + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + for (let i = 0; i < 3; i++) { + await runner.makeRequestAndWaitForEnvelope( + 'get', + '/no-cache/dedupe?id=dedupe-fresh', + errorEventExpectation('Same error', CAPTURE_MECHANISM), + ); + } +}); + +// A cached client outlives the invocation that created it, so this checks that reusing it does not +// also start reusing the isolation scope `setTag`/`setUser` write to. The uncached counterpart of +// this test lives in the `durable-object-scope` suite. +it('cacheClient: true - two consecutive invocations get different isolation scopes', async ({ signal }) => { + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + await runner.makeRequestAndWaitForEnvelope('get', '/cache/scope?id=scope-shared&seed=1', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Cache scope seed'); + // Guards the probe assertions below against passing vacuously. + expect(event.tags).toEqual(expect.objectContaining({ seeded_tag: 'from-seeding-call' })); + expect(event.user).toEqual({ id: 'user-from-seeding-call' }); + }); + + await runner.makeRequestAndWaitForEnvelope('get', '/cache/scope?id=scope-shared&seed=0', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Cache scope probe'); + expect(event.tags?.seeded_tag).toBeUndefined(); + expect(event.user).toBeUndefined(); + }); +}); + +it('cacheClient: true - streaming response works with shared client', async ({ signal }) => { + // A streamed request produces two span envelopes — the DO's own `GET /streaming` and the + // outer worker's `GET /cache/streaming` — and their arrival order is not guaranteed. Accept + // either, since the point is that spans still reach the transport at all. + const streamingSpanExpectation = (envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as { items?: Array<{ name?: string }> }; + expect(payload.items?.map(span => span.name)).toEqual( + expect.arrayContaining([expect.stringMatching(/^GET \/(cache\/)?streaming$/)]), + ); + }; + + const runner = createRunner(__dirname).start(signal); + + // Waiting per request keeps the runner alive for the second one: with both + // expectations queued up front it completes on the first request's spans and + // tears down before the second is sent. + for (let i = 0; i < 2; i++) { + const text = await runner.makeRequestAndWaitForEnvelope( + 'get', + '/cache/streaming', + streamingSpanExpectation, + ); + expect(text).toBe('chunk1chunk2'); + } +}); + +// Sync KV and SQL instrumentation produces child `db` spans inside the Durable Object. A cached +// client never reaches an invocation-boundary flush, so these only arrive if the eager drain +// covers spans too — the uncached mode is the control that the routes themselves are sound. +describe('durable object storage spans', () => { + // span-v2 wraps every attribute value as `{ value, type }`. + type SpanV2 = { name?: string; attributes?: Record }; + + const dbSpanNames = (envelope: Envelope): string[] => { + const payload = envelope[1]?.[0]?.[1] as { items?: SpanV2[] }; + return (payload.items ?? []) + .filter(span => span.attributes?.['db.system.name']?.value === 'cloudflare-durable-object-sql') + .map(span => span.name ?? ''); + }; + + for (const mode of ['cache', 'no-cache'] as const) { + it(`cacheClient: ${mode === 'cache'} - db spans are delivered`, async ({ signal }) => { + // The DO's span envelope and the outer worker's arrive in either order, so match + // unordered rather than asserting on whichever comes first. + const runner = createRunner(__dirname) + .expect((envelope: Envelope) => { + expect(dbSpanNames(envelope)).toEqual([ + 'durable_object_storage_kv_put', + 'durable_object_storage_kv_get', + 'durable_object_storage_kv_list', + 'durable_object_storage_kv_delete', + 'CREATE TABLE users', + 'INSERT users', + 'SELECT users', + ]); + }) + .unordered() + .start(signal); + + await runner.makeRequest('get', `/${mode}/storage?id=storage-${mode}`); + await runner.completed(); + }); + } +}); + +it('cacheClient: true - multiple DO instances share the same client', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect(errorEventExpectation('Cache DO handler error from instance-1', DO_MECHANISM)) + .expect(errorEventExpectation('Cache DO handler error from instance-2', DO_MECHANISM)) + .unordered() + .start(signal); + + // Two different DO instances — both should capture errors + await runner.makeRequest('get', '/cache/handler-error?id=instance-1', { expectError: true }); + await runner.makeRequest('get', '/cache/handler-error?id=instance-2', { expectError: true }); + await runner.completed(); +}); + +// The worker-side half of #22545: work registered via ctx.waitUntil finishes after +// the response and after the invocation's flush point, so the spans/log/metric/error +// are only delivered because the cached client drains them eagerly. +it('cacheClient: true - post-response waitUntil work delivers spans, log, metric and error', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect(spanEnvelopeExpectation('checkout.post-response')) + .expect(spanEnvelopeExpectation('checkout.notify-webhook')) + .expect(errorEventExpectation('Webhook delivery failed', CAPTURE_MECHANISM)) + .expect((envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as { items?: Array<{ body?: string }> }; + expect(payload.items?.some(log => log.body === 'checkout post-response log')).toBe(true); + }) + .expect((envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as { items?: Array<{ name?: string }> }; + expect(payload.items?.some(metric => metric.name === 'checkout.processed')).toBe(true); + }) + .unordered() + .start(signal); + + const text = await runner.makeRequest('get', '/post-response?id=checkout-1'); + expect(text).toBe('checkout accepted'); + await runner.completed(); +}); + +// One request fans out into N sequential DO RPC calls. Each RPC span must be +// delivered and must belong to the worker request's trace (RPC trace propagation). +it('cacheClient: true - burst DO RPC span shares the worker request trace', async ({ signal }) => { + let workerTraceId: string | undefined; + const echoTraceIds = new Set(); + + const runner = createRunner(__dirname) + .expect((envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as SpanV2Payload; + const echoSpans = (payload.items ?? []).filter(span => span.name === 'echo'); + expect(echoSpans.length).toBeGreaterThan(0); + for (const span of echoSpans) { + expect(span.attributes?.['sentry.op']?.value).toBe('rpc'); + expect(span.trace_id).toBeDefined(); + echoTraceIds.add(span.trace_id!); + } + }) + .expect((envelope: Envelope) => { + const payload = envelope[1]?.[0]?.[1] as SpanV2Payload; + const root = payload.items?.find(span => span.name === 'GET /burst'); + expect(root).toBeDefined(); + expect(root?.attributes?.['sentry.op']?.value).toBe('http.server'); + workerTraceId = root?.trace_id; + }) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/burst?n=1&id=fanout'); + await runner.completed(); + + expect(workerTraceId).toBeDefined(); + expect(echoTraceIds.size).toBe(1); + expect([...echoTraceIds][0]).toBe(workerTraceId); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/cache-client/wrangler.jsonc new file mode 100644 index 000000000000..55491bcfc34d --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/wrangler.jsonc @@ -0,0 +1,18 @@ +{ + "name": "cache-client-test", + "compatibility_date": "2025-06-17", + "main": "index.ts", + "compatibility_flags": ["nodejs_compat"], + "durable_objects": { + "bindings": [ + { "name": "CACHE_DO", "class_name": "CacheDurableObject" }, + { "name": "NO_CACHE_DO", "class_name": "NoCacheDurableObject" }, + ], + }, + "migrations": [ + { + "tag": "v1", + "new_sqlite_classes": ["CacheDurableObject", "NoCacheDurableObject"], + }, + ], +} diff --git a/packages/cloudflare/src/baseSdk.ts b/packages/cloudflare/src/baseSdk.ts index 4ffc248aad1a..cdea4dc5775b 100644 --- a/packages/cloudflare/src/baseSdk.ts +++ b/packages/cloudflare/src/baseSdk.ts @@ -81,12 +81,17 @@ export function getBaseDefaultIntegrations(options: CloudflareOptions): Integrat export function initWithDefaultIntegrations( options: CloudflareOptions, getDefaultIntegrationsImpl: (options: CloudflareOptions) => Integration[], + { skipFlushLock = false }: { skipFlushLock?: boolean } = {}, ): CloudflareClient | undefined { if (options.defaultIntegrations === undefined) { options.defaultIntegrations = getDefaultIntegrationsImpl(options); } - const flushLock = options.ctx ? makeFlushLock(options.ctx) : undefined; + // A cached client outlives any single invocation, so binding it to one + // invocation's flush lock would make later flushes wait on that invocation's + // waitUntil work forever. Eager delivery replaces the flush lock's purpose. + const invocationContext = options.ctx; + const flushLock = !skipFlushLock && invocationContext ? makeFlushLock(invocationContext) : undefined; delete options.ctx; const clientOptions: CloudflareClientOptions = { @@ -98,6 +103,7 @@ export function initWithDefaultIntegrations( // provider. Scope isolation is handled by the entrypoint wrappers' AsyncLocalStorage strategy. enableOpenTelemetrySetup: options.enableOpenTelemetrySetup ?? false, flushLock, + invocationContext, }; /*! rollup-include-development-only */ diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 85e47fbee3d7..2ba0564ef76d 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -1,6 +1,8 @@ -import type { ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core'; +import type { Client, ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core'; import { _INTERNAL_clearAiProviderSkips, + _INTERNAL_flushLogsBuffer, + _INTERNAL_flushMetricsBuffer, applySdkMetadata, debug, ServerRuntimeClient, @@ -9,7 +11,9 @@ import { import { DEBUG_BUILD } from './debug-build'; import type { ExecutionContextCompat } from './executionContext'; import type { makeFlushLock } from './flush'; +import { getOriginalWaitUntil } from './flush'; import type { CloudflareTransportOptions } from './transport'; +import { getInvocationState } from './utils/invocationContext'; /** * The Sentry Cloudflare SDK Client. @@ -26,6 +30,26 @@ export class CloudflareClient extends ServerRuntimeClient { private _unsubscribeSpanStart: (() => void) | null = null; private _unsubscribeSpanEnd: (() => void) | null = null; + private _invocationContext: ExecutionContextCompat | undefined; + + /** + * Whether this client is a cached, cross-invocation client (`cacheClient`). + * Cached clients are never disposed at an invocation boundary, so their spans/events + * are delivered eagerly instead of waiting for a per-invocation flush. + */ + public readonly isCachedClient: boolean; + + /** + * Points the client at the execution context of the invocation currently being + * served. Called on every invocation for cached clients, since they outlive any + * single invocation. Only a fallback: under concurrency the correct context is + * resolved from the invocation's async context instead (see + * `getInvocationState`), which this field cannot disambiguate. + */ + public setExecutionContext(ctx: ExecutionContextCompat | undefined): void { + this._invocationContext = ctx; + } + /** * Creates a new Cloudflare SDK instance. * @param options Configuration options for this SDK. @@ -33,7 +57,7 @@ export class CloudflareClient extends ServerRuntimeClient { public constructor(options: CloudflareClientOptions) { applySdkMetadata(options, 'cloudflare'); options._metadata = options._metadata || {}; - const { flushLock, ...serverOptions } = options; + const { flushLock, invocationContext, ...serverOptions } = options; const clientOptions: ServerRuntimeClientOptions = { ...serverOptions, @@ -46,41 +70,55 @@ export class CloudflareClient extends ServerRuntimeClient { super(clientOptions); this._flushLock = flushLock; + this._invocationContext = invocationContext; + this.isCachedClient = options.cacheClient === true; - // Track span lifecycle to know when to flush - this._unsubscribeSpanStart = this.on('spanStart', span => { - const spanId = span.spanContext().spanId; - DEBUG_BUILD && debug.log('[CloudflareClient] Span started:', spanId); + if (this.isCachedClient) { + this._setupEagerEnvelopeDelivery(); + this._setupEagerSpanDelivery(); + this._setupEagerLogAndMetricDelivery(); + } - // Negatively sampled spans never emit spanEnd, - // so tracking them would cause _pendingSpans to grow unboundedly. - // We should fix the inconsistent behavior for NonRecordingSpans in the future but - // for now, we just ignore them. - if (!spanIsSampled(span)) { - return; - } + // Track span lifecycle to know when to flush. Skipped for cached clients + // (`cacheClient`): they are never disposed, so spans that end after + // a flush are still delivered. Per-invocation clients are disposed right after + // the boundary flush, so the flush must wait for open spans to end otherwise + // their transaction never gets emitted. + if (!this.isCachedClient) { + this._unsubscribeSpanStart = this.on('spanStart', span => { + const spanId = span.spanContext().spanId; + DEBUG_BUILD && debug.log('[CloudflareClient] Span started:', spanId); - this._pendingSpans.add(spanId); + // Negatively sampled spans never emit spanEnd, + // so tracking them would cause _pendingSpans to grow unboundedly. + // We should fix the inconsistent behavior for NonRecordingSpans in the future but + // for now, we just ignore them. + if (!spanIsSampled(span)) { + return; + } - if (!this._spanCompletionPromise) { - this._spanCompletionPromise = new Promise(resolve => { - this._resolveSpanCompletion = resolve; - }); - } - }); + this._pendingSpans.add(spanId); - this._unsubscribeSpanEnd = this.on('spanEnd', span => { - const spanId = span.spanContext().spanId; - DEBUG_BUILD && debug.log('[CloudflareClient] Span ended:', spanId); - this._pendingSpans.delete(spanId); + if (!this._spanCompletionPromise) { + this._spanCompletionPromise = new Promise(resolve => { + this._resolveSpanCompletion = resolve; + }); + } + }); - // If no more pending spans, resolve the completion promise - if (this._pendingSpans.size === 0 && this._resolveSpanCompletion) { - DEBUG_BUILD && debug.log('[CloudflareClient] All spans completed, resolving promise'); - this._resolveSpanCompletion(); - this._resetSpanCompletionPromise(); - } - }); + this._unsubscribeSpanEnd = this.on('spanEnd', span => { + const spanId = span.spanContext().spanId; + DEBUG_BUILD && debug.log('[CloudflareClient] Span ended:', spanId); + this._pendingSpans.delete(spanId); + + // If no more pending spans, resolve the completion promise + if (this._pendingSpans.size === 0 && this._resolveSpanCompletion) { + DEBUG_BUILD && debug.log('[CloudflareClient] All spans completed, resolving promise'); + this._resolveSpanCompletion(); + this._resetSpanCompletionPromise(); + } + }); + } } /** @@ -93,10 +131,36 @@ export class CloudflareClient extends ServerRuntimeClient { * @return {Promise} A promise that resolves to a boolean indicating whether the flush operation was successful. */ public async flush(timeout?: number): Promise { + // Mark this invocation as past its natural flush point: anything captured from + // now on (post-response waitUntil work, detached continuations) has no later + // flush to ride, so it is delivered eagerly (see _setupEagerSpanDelivery). + const invocationState = getInvocationState(); + if (invocationState) { + invocationState.flushPointReached = true; + } + + // Wait for user waitUntil-registered work to settle before draining, so events + // captured in that work are still in the buffer. Without this the final flush + // can drain (and the client be disposed) before background captures land. if (this._flushLock) { await this._flushLock.finalize(); } + // The eager log/metric drain is debounced to a microtask, so captured logs and + // metrics may not be envelopes yet. Draining only the transport would resolve + // while they are still buffer entries — and a resolving boundary flush lets the + // invocation end before their envelopes are ever created. + if (this.isCachedClient) { + _INTERNAL_flushLogsBuffer(this); + _INTERNAL_flushMetricsBuffer(this); + } + + // Await only drains owned by this invocation. Concurrent invocations keep + // independent chains on their own isolation scopes. + if (invocationState?.eagerFlushPromise) { + await invocationState.eagerFlushPromise; + } + if (this._pendingSpans.size > 0 && this._spanCompletionPromise) { DEBUG_BUILD && debug.log('[CloudflareClient] Waiting for', this._pendingSpans.size, 'pending spans to complete...'); @@ -164,6 +228,164 @@ export class CloudflareClient extends ServerRuntimeClient { this._spanCompletionPromise = null; this._resolveSpanCompletion = null; } + + /** + * Drains the transport after an envelope has been accepted. + * + * The Cloudflare transport queues request producers until `flush()` is called. Cached + * clients cannot rely on a later invocation boundary, so each accepted envelope starts + * an eager drain. Drains are serialized per invocation and registered with that + * invocation's `waitUntil`, ensuring the runtime keeps their fetches alive after the + * response is returned. + */ + private _setupEagerEnvelopeDelivery(): void { + this.on('afterEnvelope', () => { + const transport = this.getTransport(); + if (!transport) { + return; + } + const invocationState = getInvocationState(); + const flushTransport = (): PromiseLike => transport.flush(2000); + const flushPromise = invocationState?.eagerFlushPromise + ? Promise.resolve(invocationState.eagerFlushPromise).then(flushTransport, flushTransport) + : flushTransport(); + + if (invocationState) { + invocationState.eagerFlushPromise = flushPromise; + void Promise.resolve(flushPromise).finally(() => { + if (invocationState.eagerFlushPromise === flushPromise) { + invocationState.eagerFlushPromise = undefined; + } + }); + } + + this._registerWithInvocationWaitUntil(flushPromise); + }); + } + + /** + * Delivers spans that end after the invocation's flush point. + * + * Spans ending while the invocation is in flight batch in the span buffer and + * are drained by the boundary `flush()` — nothing to do here. Spans ending + * after it (in `waitUntil` work or detached continuations) have no later + * natural flush point, and the buffer's own 5s flush timer would fire outside + * any invocation, where the send can only be registered with a stale execution + * context (or none), and the runtime suspends it. Those traces are flushed + * directly — only their own bucket, never the whole buffer, so a fan-out of + * concurrent traces stays one envelope per trace. + * + * The flush point is per invocation. In Durable Objects it lands at RPC-method + * settle, so RPC spans (which end before it) keep batching one envelope per + * trace — flushing them per call would turn a fan-out trace into one envelope + * per RPC — while detached continuations inheriting that invocation's state + * flush eagerly. + */ + private _setupEagerSpanDelivery(): void { + this.on('afterSpanEnd', span => { + const invocationState = getInvocationState(); + // Only deliver spans that end after the invocation's flush point — spans + // ending before it batch in the buffer and are drained by the boundary + // flush. RPC sub-invocations in Durable Objects never reach a flush point, + // so their spans batch one envelope per trace here. + if (!invocationState?.flushPointReached || invocationState.spanFlushScheduled) { + return; + } + invocationState.spanFlushScheduled = true; + // The trace id must come from the span, not the current scope: `continueTrace` + // writes the propagation context to the *current* scope, so the forked + // isolation scope's propagation context carries a different trace id and + // flushing by it silently no-ops (measured: ~40% of post-flush traces lost). + const traceId = span.spanContext().traceId; + // Defer to a microtask: a synchronous flush here runs before the span + // streaming integration's own `afterSpanEnd` handler has added the + // triggering span to the buffer (it is registered after this one), so the + // tail span of the invocation would be left behind. The microtask still + // runs in the same async context, so the send stays attributed to this + // invocation. + queueMicrotask(() => { + invocationState.spanFlushScheduled = false; + this.emit('flushTraceSpans', traceId); + }); + }); + } + + /** + * Turns log and metric captures into envelopes without waiting for a flush. + * + * Unlike events, logs and metrics batch client-side and only become an envelope when + * their buffer is drained. The idle drain timer is disabled for this runtime + * (`_flushInterval: 0`), and a cached client never reaches an invocation-boundary + * `flush()`, so without this a captured log or metric is never delivered at all. + * + * The buffers are drained directly rather than via `emit('flush')`, which would also + * flush an opt-in span buffer mid-invocation and fragment span segments. Draining is + * debounced to a microtask so a synchronous burst (e.g. a loop of `logger` calls) + * still produces a single envelope. + */ + private _setupEagerLogAndMetricDelivery(): void { + let scheduled = false; + const scheduleDrain = (): void => { + if (scheduled) { + return; + } + scheduled = true; + queueMicrotask(() => { + scheduled = false; + _INTERNAL_flushLogsBuffer(this); + _INTERNAL_flushMetricsBuffer(this); + }); + }; + + this.on('afterCaptureLog', scheduleDrain); + this.on('afterCaptureMetric', scheduleDrain); + } + + /** + * Registers every envelope send as tracked I/O with the capturing invocation's + * `waitUntil`. + * + * The SDK never awaits `sendEnvelope()` promises, so an envelope's fetch can be + * pending-but-untracked when the invocation's tracked work settles the runtime + * suspends it and the envelope is lost even though the send started while the + * invocation was still open. This is the dominant loss path for the last captures + * of an invocation (the root span, post-response `waitUntil` work). + */ + public override sendEnvelope(envelope: Parameters[0]): ReturnType { + const sendPromise = super.sendEnvelope(envelope); + if (this.isCachedClient) { + this._registerWithInvocationWaitUntil(sendPromise); + } + return sendPromise; + } + + /** + * Attaches a promise to the `waitUntil` of the invocation that owns the current + * async context. The invocation state identifies that invocation even under + * concurrency the fallback field would point at whichever invocation last + * called `init()`, which is the wrong one when invocations overlap. In Durable + * Objects `waitUntil` is a no-op, so this degrades to the same fire-and-forget + * behavior as before there. + */ + private _registerWithInvocationWaitUntil(promise: PromiseLike): void { + const ctx = getInvocationState()?.ctx ?? this._invocationContext; + + if (!ctx) { + return; + } + + try { + getOriginalWaitUntil(ctx)?.call( + ctx, + Promise.resolve(promise).then( + () => undefined, + () => undefined, + ), + ); + } catch { + // The owning invocation already ended; the send races isolate teardown either way. + } + } } interface BaseCloudflareOptions { @@ -299,6 +521,24 @@ interface BaseCloudflareOptions { * IMPORTANT: Only set this option to `true` while developing, not in production! */ spotlight?: boolean | string; + + /** + * Cache the client and reuse it across invocations within the same isolate. + * + * The SDK creates one client per isolate and reuses it for all requests/DO + * handlers in that isolate. This avoids the per-invocation cost of + * constructing a new client. + * + * Since a cached client outlives any single invocation, delivery cannot rely + * on end-of-invocation flushes: captured events are flushed eagerly as they are + * captured, so data captured in detached/background work is still delivered. + * + * When disabled, a new client is created per invocation and disposed after the + * handler completes. + * + * @default true + */ + cacheClient?: boolean; } /** @@ -317,4 +557,5 @@ export interface CloudflareOptions extends Options, */ export interface CloudflareClientOptions extends ClientOptions, BaseCloudflareOptions { flushLock?: ReturnType; + invocationContext?: ExecutionContextCompat; } diff --git a/packages/cloudflare/src/clientCache.ts b/packages/cloudflare/src/clientCache.ts new file mode 100644 index 000000000000..2a604f36f0f0 --- /dev/null +++ b/packages/cloudflare/src/clientCache.ts @@ -0,0 +1,23 @@ +import { GLOBAL_OBJ } from '@sentry/core'; +import type { CloudflareClient } from './client'; + +const GLOBAL_CLIENT_KEY = '__SENTRY_CLOUDFLARE_CLIENT__' as const; + +type GlobalWithCloudflareClient = typeof GLOBAL_OBJ & { + [GLOBAL_CLIENT_KEY]?: CloudflareClient; +}; + +/** Returns the one cached Cloudflare client for this isolate. */ +export function getCachedClient(): CloudflareClient | undefined { + return (GLOBAL_OBJ as GlobalWithCloudflareClient)[GLOBAL_CLIENT_KEY]; +} + +/** Stores the one Cloudflare client reused by every invocation in this isolate. */ +export function cacheClient(client: CloudflareClient): void { + (GLOBAL_OBJ as GlobalWithCloudflareClient)[GLOBAL_CLIENT_KEY] = client; +} + +/** @hidden Only for testing - clears the isolate's cached Cloudflare client. */ +export function _clearGlobalClientCache(): void { + (GLOBAL_OBJ as GlobalWithCloudflareClient)[GLOBAL_CLIENT_KEY] = undefined; +} diff --git a/packages/cloudflare/src/flush.ts b/packages/cloudflare/src/flush.ts index fe86e21dbd62..8273f1c318da 100644 --- a/packages/cloudflare/src/flush.ts +++ b/packages/cloudflare/src/flush.ts @@ -119,6 +119,8 @@ function getOrCreateFlushLockRegistry(context: ExecutionContextCompat): FlushLoc /** * 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. + * Cached clients (`cacheClient`) are reused across invocations, so + * they are flushed but not disposed. * * This function never rejects. On Workers, a rejected promise passed to * `ctx.waitUntil` marks the whole invocation as `outcome: exception` even when @@ -139,10 +141,14 @@ export async function flushAndDispose(client: Client | undefined, timeout = 2000 } catch (e) { DEBUG_BUILD && debug.warn('Failed to flush client', e); } finally { - try { - client?.dispose(); - } catch (e) { - DEBUG_BUILD && debug.warn('Failed to dispose client', e); + // Only dispose per-invocation clients. Cached clients (`cacheClient`) + // are reused across invocations and must not be disposed at an invocation boundary. + if (!(client as { isCachedClient?: boolean } | undefined)?.isCachedClient) { + try { + client?.dispose(); + } catch (e) { + DEBUG_BUILD && debug.warn('Failed to dispose client', e); + } } } } diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentEmail.ts b/packages/cloudflare/src/instrumentations/worker/instrumentEmail.ts index ed2d9a05f7f5..4fcc477a5082 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentEmail.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentEmail.ts @@ -17,6 +17,7 @@ import { getFinalOptions } from '../../options'; import { addCloudResourceContext } from '../../scope-utils'; import { init } from '../../sdk'; import { instrumentContext } from '../../utils/instrumentContext'; +import { setInvocationState } from '../../utils/invocationContext'; import { instrumentEnv } from './instrumentEnv'; /** @@ -31,6 +32,8 @@ function wrapEmailHandler( return withIsolationScope(isolationScope => { const waitUntil = context.waitUntil.bind(context); + setInvocationState(isolationScope, { ctx: context }); + const client = init({ ...options, ctx: context }); isolationScope.setClient(client); diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentQueue.ts b/packages/cloudflare/src/instrumentations/worker/instrumentQueue.ts index ac609e241a55..975bc1a986dc 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentQueue.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentQueue.ts @@ -17,6 +17,7 @@ import { getFinalOptions } from '../../options'; import { addCloudResourceContext } from '../../scope-utils'; import { init } from '../../sdk'; import { instrumentContext } from '../../utils/instrumentContext'; +import { setInvocationState } from '../../utils/invocationContext'; import { instrumentEnv } from './instrumentEnv'; /** @@ -31,6 +32,8 @@ function wrapQueueHandler( return withIsolationScope(isolationScope => { const waitUntil = context.waitUntil.bind(context); + setInvocationState(isolationScope, { ctx: context }); + const client = init({ ...options, ctx: context }); isolationScope.setClient(client); diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts b/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts index 018dd8b56ee1..dd53c41c1b99 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts @@ -17,6 +17,7 @@ import { getFinalOptions } from '../../options'; import { addCloudResourceContext } from '../../scope-utils'; import { init } from '../../sdk'; import { instrumentContext } from '../../utils/instrumentContext'; +import { setInvocationState } from '../../utils/invocationContext'; import { instrumentEnv } from './instrumentEnv'; function wrapScheduledHandler( @@ -28,6 +29,8 @@ function wrapScheduledHandler( return withIsolationScope(isolationScope => { const waitUntil = context.waitUntil.bind(context); + setInvocationState(isolationScope, { ctx: context }); + const client = init({ ...options, ctx: context }); isolationScope.setClient(client); diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentTail.ts b/packages/cloudflare/src/instrumentations/worker/instrumentTail.ts index 925f2b504605..31ecd6674120 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentTail.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentTail.ts @@ -9,6 +9,7 @@ import { getFinalOptions } from '../../options'; import { addCloudResourceContext } from '../../scope-utils'; import { init } from '../../sdk'; import { instrumentContext } from '../../utils/instrumentContext'; +import { setInvocationState } from '../../utils/invocationContext'; import { instrumentEnv } from './instrumentEnv'; /** @@ -19,6 +20,8 @@ function wrapTailHandler(options: CloudflareOptions, context: ExecutionContext, return withIsolationScope(async isolationScope => { const waitUntil = context.waitUntil.bind(context); + setInvocationState(isolationScope, { ctx: context }); + const client = init({ ...options, ctx: context }); isolationScope.setClient(client); diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index 07d5435e4084..0ef61022b060 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -228,5 +228,5 @@ export function wrapRequestHandlerWithInit( }); }, ); - }); + }, wrapperOptions.context); } diff --git a/packages/cloudflare/src/sdk.ts b/packages/cloudflare/src/sdk.ts index 12bcb420d788..d940e58c8845 100644 --- a/packages/cloudflare/src/sdk.ts +++ b/packages/cloudflare/src/sdk.ts @@ -1,6 +1,11 @@ import type { Integration } from '@sentry/core'; +import { getCurrentScope, setCurrentClient } from '@sentry/core'; import { getBaseDefaultIntegrations, initWithDefaultIntegrations } from './baseSdk'; import type { CloudflareClient, CloudflareOptions } from './client'; +import { cacheClient, getCachedClient } from './clientCache'; + +// Test-only helper, re-exported here so tests can reset the global client cache. +export { _clearGlobalClientCache } from './clientCache'; /** * Get the default integrations for the Cloudflare SDK. @@ -11,7 +16,46 @@ export function getDefaultIntegrations(options: CloudflareOptions): Integration[ /** * Initializes the cloudflare SDK. + * + * The client is cached and reused across invocations within the same isolate, + * unless `cacheClient: false` is passed. This avoids the + * per-invocation cost of constructing a new client, and it is what makes + * Durable Object telemetry reliable: a per-invocation client is disposed at + * the end of the handler, and in a Durable Object there is no `waitUntil` + * boundary that reliably extends execution, so spans/events that end after + * disposal would otherwise be lost. */ export function init(options: CloudflareOptions): CloudflareClient | undefined { - return initWithDefaultIntegrations(options, getDefaultIntegrations); + const cacheEnabled = options.cacheClient !== false; + + if (cacheEnabled) { + // Normalize the flag so the client marks itself as cached. + options.cacheClient = true; + } + + if (cacheEnabled && options.dsn) { + const cached = getCachedClient(); + // A cached client that has lost its transport was disposed. Replace it rather + // than returning a dead client for the rest of the isolate's lifetime. + if (cached?.getTransport()) { + // Mirror the two scope side effects of `initAndBind`, which only runs on first + // creation. Without the re-bind the scope keeps whatever client a previous init + // left behind — which may have been disposed since — and without the update + // `initialScope` would apply only to an isolate's very first invocation. + getCurrentScope().update(options.initialScope); + setCurrentClient(cached); + // The cached client outlives the invocation that created it, so its eager + // sends must be registered with the current invocation's waitUntil. + cached.setExecutionContext(options.ctx); + return cached; + } + } + + const client = initWithDefaultIntegrations(options, getDefaultIntegrations, { skipFlushLock: cacheEnabled }); + + if (cacheEnabled && client && options.dsn) { + cacheClient(client); + } + + return client; } diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts index 6069ec631189..685cb7fbccd5 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -8,7 +8,7 @@ export interface CloudflareTransportOptions extends BaseTransportOptions { fetchOptions?: RequestInit; } -const DEFAULT_TRANSPORT_BUFFER_SIZE = 30; +const DEFAULT_TRANSPORT_BUFFER_SIZE = 256; /** * This is a modified promise buffer that collects tasks until drain is called. diff --git a/packages/cloudflare/src/utils/invocationContext.ts b/packages/cloudflare/src/utils/invocationContext.ts new file mode 100644 index 000000000000..ec1f450bb885 --- /dev/null +++ b/packages/cloudflare/src/utils/invocationContext.ts @@ -0,0 +1,72 @@ +import type { Scope } from '@sentry/core'; +import { getDefaultIsolationScope, getIsolationScope } from '@sentry/core'; +import type { ExecutionContextCompat } from '../executionContext'; + +/** + * State owned by a single invocation (request, RPC call, cron, ...). + * + * A cached client (`cacheClient`) is shared by all invocations running in the same + * isolate, so anything invocation-owned must not live on the client: two overlapping + * requests would otherwise overwrite each other's state (e.g. the execution context + * used to register eager flushes) and the earlier invocation's envelopes would be + * suspended when the later invocation ends. + * + * The state rides on the invocation's forked isolation scope, which the async + * context strategy (AsyncLocalStorage) hands back for exactly the async context that + * owns the invocation — including detached continuations created inside it. A symbol + * key keeps it off `Scope.clone()` and out of serialized event data. + */ +export interface InvocationState { + /** + * The execution context of the invocation that owns this scope. Eager envelope + * sends are registered with this context's `waitUntil`, so they are attributed to + * the invocation that captured the data — even when a concurrent invocation has + * since pointed the shared client at its own context. + */ + readonly ctx: ExecutionContextCompat | undefined; + /** + * Set by `CloudflareClient.flush()` — the invocation's natural flush point. + * Spans ending before it are drained by that flush; spans ending after it (in + * `waitUntil` work or detached continuations) have no later flush to ride and + * are delivered eagerly. + */ + flushPointReached?: boolean; + /** + * Set while an eager span flush is scheduled for this invocation. Kept per + * invocation (not on the shared client) so two concurrent invocations past + * their flush point each schedule their own flush in their own async context — + * the flush and its envelope send stay attributed to the owning invocation. + */ + spanFlushScheduled?: boolean; + /** Eager transport drains owned by this invocation, serialized in capture order. */ + eagerFlushPromise?: PromiseLike; +} + +const INVOCATION_STATE: unique symbol = Symbol('sentryInvocationState'); + +type ScopeWithInvocationState = Scope & { + [INVOCATION_STATE]?: InvocationState; +}; + +/** + * Attaches invocation state to a forked isolation scope. Only meaningful on a scope + * that outlives nothing but this invocation — never attach to the default isolation + * scope, which is shared by every invocation in the isolate. + */ +export function setInvocationState(scope: Scope, state: InvocationState): void { + (scope as ScopeWithInvocationState)[INVOCATION_STATE] = state; +} + +/** + * Returns the state of the invocation that owns the current async context, or + * `undefined` outside any instrumented invocation (the default isolation scope is + * shared, so state read from it could not be attributed to one invocation — and it + * is never attached there in the first place). + */ +export function getInvocationState(): InvocationState | undefined { + const isolationScope = getIsolationScope() as ScopeWithInvocationState; + if (isolationScope === getDefaultIsolationScope()) { + return undefined; + } + return isolationScope[INVOCATION_STATE]; +} diff --git a/packages/cloudflare/src/utils/invocationScope.ts b/packages/cloudflare/src/utils/invocationScope.ts index 3591bac64eb2..aaaf98fdaa2a 100644 --- a/packages/cloudflare/src/utils/invocationScope.ts +++ b/packages/cloudflare/src/utils/invocationScope.ts @@ -1,4 +1,6 @@ import { getDefaultIsolationScope, getIsolationScope, type Scope, withIsolationScope } from '@sentry/core'; +import type { ExecutionContextCompat } from '../executionContext'; +import { setInvocationState } from './invocationContext'; /** * Runs `callback` on the isolation scope for the current invocation. @@ -21,10 +23,15 @@ import { getDefaultIsolationScope, getIsolationScope, type Scope, withIsolationS * default scope even inside an invocation; there the fork degrades to a no-op, which the stack strategy * tolerates. This matches the approach used by `patchEventHandler` in Nuxt. */ -export function withInvocationIsolationScope(callback: (scope: Scope) => T): T { +export function withInvocationIsolationScope(callback: (scope: Scope) => T, context?: ExecutionContextCompat): T { const isolationScope = getIsolationScope(); - const newIsolationScope = isolationScope === getDefaultIsolationScope() ? isolationScope.clone() : isolationScope; + const isEntryPoint = isolationScope === getDefaultIsolationScope(); + const newIsolationScope = isEntryPoint ? isolationScope.clone() : isolationScope; + + if (isEntryPoint) { + setInvocationState(newIsolationScope, { ctx: context }); + } return withIsolationScope(newIsolationScope, () => callback(newIsolationScope)); } diff --git a/packages/cloudflare/src/wrapMethodWithSentry.ts b/packages/cloudflare/src/wrapMethodWithSentry.ts index 290c9947b5e0..e15d4cd7d5b6 100644 --- a/packages/cloudflare/src/wrapMethodWithSentry.ts +++ b/packages/cloudflare/src/wrapMethodWithSentry.ts @@ -234,7 +234,10 @@ export function wrapMethodWithSentry( return executeSpan(); }; - return withInvocationIsolationScope(wrappedFunction); + return withInvocationIsolationScope( + wrappedFunction, + wrapperOptions.context as ExecutionContextCompat | undefined, + ); }, }), noMark, diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 7e305f6aec65..3bb3c542453c 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -2,6 +2,8 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { setAsyncLocalStorageAsyncContextStrategy } from '@sentry/server-utils/no-diagnostic-channels'; import { CloudflareClient, type CloudflareClientOptions } from '../src/client'; import { makeFlushLock } from '../src/flush'; +import { getInvocationState } from '../src/utils/invocationContext'; +import { withInvocationIsolationScope } from '../src/utils/invocationScope'; const TRACE_FLAG_SAMPLED = 0x1; @@ -9,6 +11,8 @@ const MOCK_CLIENT_OPTIONS: CloudflareClientOptions = { dsn: 'https://public@dsn.ingest.sentry.io/1337', stackParser: () => [], integrations: [], + // These tests exercise the per-invocation client behavior + cacheClient: false, transport: () => ({ send: vi.fn().mockResolvedValue({}), flush: vi.fn().mockResolvedValue(true), @@ -220,6 +224,52 @@ describe('CloudflareClient', () => { }); }); + describe('flush()', () => { + it('calls transport flush with the given timeout', async () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const privateClient = client as unknown as { + _transport: { flush: ReturnType }; + }; + + await client.flush(3000); + + expect(privateClient._transport.flush).toHaveBeenCalledWith(3000); + }); + + it('resolves with the transport flush result', async () => { + const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); + + const result = await client.flush(1000); + + expect(result).toBe(true); + }); + + it('waits for the flush lock before draining the transport', async () => { + let releaseLock!: () => void; + const finalize = vi.fn(() => new Promise(resolve => (releaseLock = resolve))); + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + flushLock: { ready: Promise.resolve(), finalize }, + }); + + const privateClient = client as unknown as { + _transport: { flush: ReturnType }; + }; + + const flushPromise = client.flush(1000); + + // The transport must not drain while the lock is pending + await Promise.resolve(); + expect(finalize).toHaveBeenCalled(); + expect(privateClient._transport.flush).not.toHaveBeenCalled(); + + releaseLock(); + await flushPromise; + expect(privateClient._transport.flush).toHaveBeenCalledWith(1000); + }); + }); + describe('span lifecycle tracking', () => { it('tracks pending spans when spanStart is emitted', () => { const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); @@ -332,5 +382,469 @@ describe('CloudflareClient', () => { client.emit('spanStart', mockSpan as any); expect(privateClient._pendingSpans.has('test-span-id')).toBe(false); }); + + it('does not track spans when cacheClient is enabled', async () => { + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + }); + + const privateClient = client as unknown as { + _pendingSpans: Set; + _unsubscribeSpanStart: (() => void) | null; + _unsubscribeSpanEnd: (() => void) | null; + }; + + // Span tracking is disabled for cached clients — flush must not wait + expect(privateClient._unsubscribeSpanStart).toBeNull(); + expect(privateClient._unsubscribeSpanEnd).toBeNull(); + + const mockSpan = { + spanContext: () => ({ spanId: 'test-span-id', traceFlags: TRACE_FLAG_SAMPLED }), + }; + client.emit('spanStart', mockSpan as any); + + expect(privateClient._pendingSpans.size).toBe(0); + await expect(client.flush(10)).resolves.toBe(true); + }); + }); + + describe('cached client eager flush tracking', () => { + function makeEagerFlushClient(flushMock: ReturnType): CloudflareClient { + return new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: flushMock, + }), + }); + } + + it('flush() awaits in-flight eager envelope flushes', async () => { + let resolveEagerFlush: (value: boolean) => void = () => undefined; + let call = 0; + const flushMock = vi.fn().mockImplementation(() => { + call++; + if (call === 1) { + return new Promise(res => { + resolveEagerFlush = res; + }); + } + return Promise.resolve(true); + }); + const client = makeEagerFlushClient(flushMock); + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + await withInvocationIsolationScope(async () => { + // An emitted envelope starts this invocation's eager transport drain. + client.emit('afterEnvelope', {}); + expect(flushMock).toHaveBeenCalledTimes(1); + + let flushResolved = false; + const flushPromise = client.flush(10).then(result => { + flushResolved = true; + return result; + }); + + // The boundary flush waits for the drain owned by this invocation. + await new Promise(resolve => setTimeout(resolve, 20)); + expect(flushResolved).toBe(false); + + resolveEagerFlush(true); + await expect(flushPromise).resolves.toBe(true); + expect(flushResolved).toBe(true); + }, ctx as never); + }); + + it('flush() resolves immediately once eager flushes have settled', async () => { + const flushMock = vi.fn().mockResolvedValue(true); + const client = makeEagerFlushClient(flushMock); + + client.emit('afterEnvelope', {}); + expect(flushMock).toHaveBeenCalledTimes(1); + + await expect(client.flush(10)).resolves.toBe(true); + }); + + it('tracks eager drains independently for concurrent invocations', async () => { + let resolveA: (value: boolean) => void = () => undefined; + let resolveB: (value: boolean) => void = () => undefined; + let call = 0; + const flushMock = vi.fn().mockImplementation(() => { + call++; + if (call === 1) { + return new Promise(resolve => { + resolveA = resolve; + }); + } + if (call === 2) { + return new Promise(resolve => { + resolveB = resolve; + }); + } + return Promise.resolve(true); + }); + const client = makeEagerFlushClient(flushMock); + const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + let flushAResolved = false; + let flushBResolved = false; + const flushA = withInvocationIsolationScope(async () => { + client.emit('afterEnvelope', {}); + return client.flush(1000).then(result => { + flushAResolved = true; + return result; + }); + }, ctxA as never); + const flushB = withInvocationIsolationScope(async () => { + client.emit('afterEnvelope', {}); + return client.flush(1000).then(result => { + flushBResolved = true; + return result; + }); + }, ctxB as never); + + resolveA(true); + await expect(flushA).resolves.toBe(true); + expect(flushAResolved).toBe(true); + expect(flushBResolved).toBe(false); + + resolveB(true); + await expect(flushB).resolves.toBe(true); + expect(flushBResolved).toBe(true); + }); + + it('does not flush eagerly per envelope when cacheClient is disabled', () => { + const flushMock = vi.fn().mockResolvedValue(true); + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: false, + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: flushMock, + }), + }); + + client.emit('afterEnvelope', {}); + expect(flushMock).not.toHaveBeenCalled(); + }); + + it('registers the eager flush with the invocation context waitUntil', () => { + const flushMock = vi.fn().mockResolvedValue(true); + const waitUntil = vi.fn(); + const ctx = { waitUntil, passThroughOnException: vi.fn() }; + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + // oxlint-disable-next-line typescript/no-explicit-any + invocationContext: ctx as any, + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: flushMock, + }), + }); + + client.emit('afterEnvelope', {}); + expect(flushMock).toHaveBeenCalledTimes(1); + expect(waitUntil).toHaveBeenCalledTimes(1); + }); + + it('uses the context from setExecutionContext for eager flush registration', () => { + const flushMock = vi.fn().mockResolvedValue(true); + const waitUntil = vi.fn(); + const ctx = { waitUntil, passThroughOnException: vi.fn() }; + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: flushMock, + }), + }); + + // oxlint-disable-next-line typescript/no-explicit-any + client.setExecutionContext(ctx as any); + client.emit('afterEnvelope', {}); + expect(waitUntil).toHaveBeenCalledTimes(1); + }); + + it('registers the eager flush with the capturing invocation, not the latest one', () => { + const flushMock = vi.fn().mockResolvedValue(true); + const waitUntilA = vi.fn(); + const waitUntilB = vi.fn(); + const ctxA = { waitUntil: waitUntilA, passThroughOnException: vi.fn() }; + const ctxB = { waitUntil: waitUntilB, passThroughOnException: vi.fn() }; + const client = makeEagerFlushClient(flushMock); + + // The shared client's fallback points at the latest invocation (B). An + // envelope captured by the still-running invocation A must register its + // flush on A's waitUntil — otherwise it is suspended when B's invocation + // ends first. + // oxlint-disable-next-line typescript/no-explicit-any + client.setExecutionContext(ctxB as any); + + withInvocationIsolationScope(() => { + client.emit('afterEnvelope', {}); + }, ctxA as never); + expect(waitUntilA).toHaveBeenCalledTimes(1); + expect(waitUntilB).not.toHaveBeenCalled(); + + withInvocationIsolationScope(() => { + client.emit('afterEnvelope', {}); + }, ctxB as never); + expect(waitUntilB).toHaveBeenCalledTimes(1); + }); + + it('registers envelope sends with the capturing invocation waitUntil', () => { + const waitUntil = vi.fn(); + const ctx = { waitUntil, passThroughOnException: vi.fn() }; + const sendMock = vi.fn().mockResolvedValue({}); + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + transport: () => ({ + send: sendMock, + flush: vi.fn().mockResolvedValue(true), + }), + }); + + withInvocationIsolationScope(() => { + void client.sendEnvelope([{}, []] as never); + }, ctx as never); + + expect(sendMock).toHaveBeenCalledTimes(1); + expect(waitUntil).toHaveBeenCalledTimes(1); + }); + + it('does not register sends with waitUntil when cacheClient is disabled', () => { + const waitUntil = vi.fn(); + const ctx = { waitUntil, passThroughOnException: vi.fn() }; + const sendMock = vi.fn().mockResolvedValue({}); + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: false, + transport: () => ({ + send: sendMock, + flush: vi.fn().mockResolvedValue(true), + }), + }); + + withInvocationIsolationScope(() => { + void client.sendEnvelope([{}, []] as never); + }, ctx as never); + + expect(sendMock).toHaveBeenCalledTimes(1); + expect(waitUntil).not.toHaveBeenCalled(); + }); + + it('never lets a failing send reject the waitUntil registration', async () => { + let registered: Promise | undefined; + const waitUntil = vi.fn((promise: Promise) => { + registered = promise; + }); + const ctx = { waitUntil, passThroughOnException: vi.fn() }; + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + transport: () => ({ + send: vi.fn().mockRejectedValue(new Error('ingest down')), + flush: vi.fn().mockResolvedValue(true), + }), + }); + + withInvocationIsolationScope(() => { + void client.sendEnvelope([{}, []] as never); + }, ctx as never); + + expect(waitUntil).toHaveBeenCalledTimes(1); + // The promise handed to the runtime must resolve — a rejected waitUntil + // promise would mark the invocation's outcome as an exception. + await expect(registered).resolves.toBeUndefined(); + }); + + it('does not register a waitUntil when no invocation context is set', () => { + const flushMock = vi.fn().mockResolvedValue(true); + const waitUntil = vi.fn(); + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: flushMock, + }), + }); + + client.emit('afterEnvelope', {}); + expect(flushMock).toHaveBeenCalledTimes(1); + expect(waitUntil).not.toHaveBeenCalled(); + }); + }); + + describe('cached client eager span delivery', () => { + function makeCachedClient(): { client: CloudflareClient; flushSpy: ReturnType } { + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: true, + traceLifecycle: 'stream', + } as never); + const flushSpy = vi.fn(); + client.on('flushTraceSpans', flushSpy); + return { client, flushSpy }; + } + + // The handler only reads the span's trace id — it is forwarded to the flushTraceSpans hook. + function makeSpan(traceId: string) { + return { spanContext: () => ({ traceId }) }; + } + + const tick = (): Promise => new Promise(resolve => setTimeout(resolve, 0)); + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + it('does not flush spans ending before the invocation flush point', async () => { + const { client, flushSpy } = makeCachedClient(); + + await withInvocationIsolationScope(async () => { + client.emit('afterSpanEnd', makeSpan('trace-b') as never); + await tick(); + }, ctx as never); + + expect(flushSpy).not.toHaveBeenCalled(); + }); + + it('flushes the trace of a span ending after the invocation flush point', async () => { + const { client, flushSpy } = makeCachedClient(); + + await withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-1') as never); + await tick(); + }, ctx as never); + + expect(flushSpy).toHaveBeenCalledTimes(1); + expect(flushSpy).toHaveBeenCalledWith('trace-1'); + }); + + it('does not flush spans of an invocation whose flush point has not been reached', async () => { + const { client, flushSpy } = makeCachedClient(); + + // Invocation A passes its flush point … + await withInvocationIsolationScope(async () => { + await client.flush(0); + }, ctx as never); + + // … while invocation B is still in flight — its spans keep batching + const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + await withInvocationIsolationScope(async () => { + client.emit('afterSpanEnd', makeSpan('trace-1') as never); + await tick(); + }, ctxB as never); + + expect(flushSpy).not.toHaveBeenCalled(); + }); + + it('schedules a flush per invocation when concurrent invocations are past their flush point', async () => { + const { client, flushSpy } = makeCachedClient(); + + const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + // Both invocations end a span past their flush point in the same tick — each + // must schedule its own flush in its own async context, so the listener + // derives the right trace (and waitUntil) per invocation. + await Promise.all([ + withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-a') as never); + }, ctxA as never), + withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-b') as never); + }, ctxB as never), + ]); + await tick(); + + expect(flushSpy).toHaveBeenCalledTimes(2); + expect(flushSpy).toHaveBeenCalledWith('trace-a'); + expect(flushSpy).toHaveBeenCalledWith('trace-b'); + }); + + it("flushes each invocation's trace in its own async context", async () => { + const { client } = makeCachedClient(); + const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const flushContext = new Map(); + + client.on('flushTraceSpans', traceId => { + flushContext.set(String(traceId), getInvocationState()?.ctx); + }); + + await Promise.all([ + withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-a') as never); + }, ctxA as never), + withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-b') as never); + }, ctxB as never), + ]); + await tick(); + + expect(flushContext.get('trace-a')).toBe(ctxA); + expect(flushContext.get('trace-b')).toBe(ctxB); + }); + + it('delivers spans of detached continuations eagerly once the owning invocation flushed', async () => { + const { client, flushSpy } = makeCachedClient(); + + let releaseDetached!: () => void; + const detachedGate = new Promise(resolve => { + releaseDetached = resolve; + }); + // A detached continuation is created inside the invocation but settles after it + const continuation = withInvocationIsolationScope(async () => { + await client.flush(0); + return (async () => { + await detachedGate; + client.emit('afterSpanEnd', makeSpan('trace-1') as never); + })(); + }, ctx as never); + + releaseDetached(); + await continuation; + await tick(); + + expect(flushSpy).toHaveBeenCalledTimes(1); + expect(flushSpy).toHaveBeenCalledWith('trace-1'); + }); + + it('does not flush spans ending outside any invocation', async () => { + const { client, flushSpy } = makeCachedClient(); + + client.emit('afterSpanEnd', makeSpan('trace-1') as never); + await tick(); + + expect(flushSpy).not.toHaveBeenCalled(); + }); + + it('does not flush for span ends when cacheClient is disabled', async () => { + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient: false, + traceLifecycle: 'stream', + } as never); + const flushSpy = vi.fn(); + client.on('flushTraceSpans', flushSpy); + + await withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-1') as never); + await tick(); + }, ctx as never); + + expect(flushSpy).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/cloudflare/test/flush.test.ts b/packages/cloudflare/test/flush.test.ts index bcef56a8c101..9686d3e7e61b 100644 --- a/packages/cloudflare/test/flush.test.ts +++ b/packages/cloudflare/test/flush.test.ts @@ -138,6 +138,19 @@ describe('flushAndDispose', () => { await expect(flushAndDispose(undefined)).resolves.toBeUndefined(); flushSpy.mockRestore(); }); + + it('should not dispose the client when it is cached (cacheClient: true)', async () => { + const mockClient = { + flush: vi.fn().mockResolvedValue(true), + dispose: vi.fn(), + isCachedClient: true, + } as unknown as Client; + + await flushAndDispose(mockClient); + + expect(mockClient.flush).toHaveBeenCalled(); + expect(mockClient.dispose).not.toHaveBeenCalled(); + }); }); describe('getOriginalWaitUntil', () => { diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentEmail.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentEmail.test.ts index ff524a76fa8f..d5bcd5e9f95c 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentEmail.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentEmail.test.ts @@ -7,6 +7,7 @@ import * as SentryCore from '@sentry/core'; import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { CloudflareClient } from '../../../src/client'; import { withSentry } from '../../../src/withSentry'; +import { resetSdk } from '../../testUtils'; const MOCK_ENV = { SENTRY_DSN: 'https://public@dsn.ingest.sentry.io/1337', @@ -44,6 +45,7 @@ function addDelayedWaitUntil(context: ExecutionContext) { describe('instrumentEmail', () => { beforeEach(() => { vi.clearAllMocks(); + resetSdk(); }); test('does not double-wrap when withSentry is called twice', async () => { @@ -289,7 +291,7 @@ describe('instrumentEmail', () => { }, } satisfies ExportedHandler; - const wrappedHandler = withSentry(vi.fn(), handler); + const wrappedHandler = withSentry(() => ({ cacheClient: false }), handler); const waits: Promise[] = []; const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.email?.(createMockEmailMessage(), MOCK_ENV_WITHOUT_DSN, { diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentFetch.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentFetch.test.ts index 29ce3f4c5948..1a0e94093444 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentFetch.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentFetch.test.ts @@ -6,6 +6,7 @@ import type { Event } from '@sentry/core'; import * as SentryCore from '@sentry/core'; import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { withSentry } from '../../../src/withSentry'; +import { resetSdk } from '../../testUtils'; const MOCK_ENV = { SENTRY_DSN: 'https://public@dsn.ingest.sentry.io/1337', @@ -30,6 +31,7 @@ function addDelayedWaitUntil(context: ExecutionContext) { describe('instrumentFetch', () => { beforeEach(() => { vi.clearAllMocks(); + resetSdk(); }); test('does not double-wrap when withSentry is called twice', async () => { @@ -160,7 +162,7 @@ describe('instrumentFetch', () => { }, } satisfies ExportedHandler; - const wrappedHandler = withSentry(vi.fn(), handler); + const wrappedHandler = withSentry(() => ({ cacheClient: false }), handler); const waits: Promise[] = []; const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentQueue.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentQueue.test.ts index 66bf3077c39e..7aa89513dcad 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentQueue.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentQueue.test.ts @@ -7,6 +7,7 @@ import * as SentryCore from '@sentry/core'; import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { CloudflareClient } from '../../../src/client'; import { withSentry } from '../../../src/withSentry'; +import { resetSdk } from '../../testUtils'; const MOCK_ENV = { SENTRY_DSN: 'https://public@dsn.ingest.sentry.io/1337', @@ -57,6 +58,7 @@ function addDelayedWaitUntil(context: ExecutionContext) { describe('instrumentQueue', () => { beforeEach(() => { vi.clearAllMocks(); + resetSdk(); }); test('does not double-wrap when withSentry is called twice', async () => { @@ -308,7 +310,7 @@ describe('instrumentQueue', () => { }, } satisfies ExportedHandler; - const wrappedHandler = withSentry(vi.fn(), handler); + const wrappedHandler = withSentry(() => ({ cacheClient: false }), handler); const waits: Promise[] = []; const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.queue?.(createMockQueueBatch(), MOCK_ENV_WITHOUT_DSN, { diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts index 2597441d249e..cd541ca1ae51 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts @@ -7,6 +7,7 @@ import * as SentryCore from '@sentry/core'; import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { CloudflareClient } from '../../../src/client'; import { withSentry } from '../../../src/withSentry'; +import { resetSdk } from '../../testUtils'; const MOCK_ENV = { SENTRY_DSN: 'https://public@dsn.ingest.sentry.io/1337', @@ -39,6 +40,7 @@ function addDelayedWaitUntil(context: ExecutionContext) { describe('instrumentScheduled', () => { beforeEach(() => { vi.clearAllMocks(); + resetSdk(); }); test('does not double-wrap when withSentry is called twice', async () => { @@ -285,7 +287,7 @@ describe('instrumentScheduled', () => { }, } satisfies ExportedHandler; - const wrappedHandler = withSentry(vi.fn(), handler); + const wrappedHandler = withSentry(() => ({ cacheClient: false }), handler); const waits: Promise[] = []; const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.scheduled?.(createMockScheduledController(), MOCK_ENV_WITHOUT_DSN, { diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentTail.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentTail.test.ts index 4f47dc3c62c7..014916e6f158 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentTail.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentTail.test.ts @@ -7,6 +7,7 @@ import * as SentryCore from '@sentry/core'; import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { CloudflareClient } from '../../../src/client'; import { withSentry } from '../../../src/withSentry'; +import { resetSdk } from '../../testUtils'; const MOCK_ENV = { SENTRY_DSN: 'https://public@dsn.ingest.sentry.io/1337', @@ -58,6 +59,7 @@ function addDelayedWaitUntil(context: ExecutionContext) { describe('instrumentTail', () => { beforeEach(() => { vi.clearAllMocks(); + resetSdk(); }); test('does not double-wrap when withSentry is called twice', async () => { @@ -260,7 +262,7 @@ describe('instrumentTail', () => { }, } satisfies ExportedHandler; - const wrappedHandler = withSentry(vi.fn(), handler); + const wrappedHandler = withSentry(() => ({ cacheClient: false }), handler); const waits: Promise[] = []; const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.tail?.(createMockTailEvent(), MOCK_ENV_WITHOUT_DSN, { diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index 44587f6224de..4c9ed540397e 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -10,6 +10,7 @@ import type { CloudflareOptions } from '../src/client'; import { CloudflareClient } from '../src/client'; import { httpServerIntegration } from '../src/integrations/httpServer'; import { wrapRequestHandler } from '../src/request'; +import { _clearGlobalClientCache, init } from '../src/sdk'; const MOCK_OPTIONS: CloudflareOptions = { dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -977,3 +978,195 @@ describe('flushAndDispose', () => { disposeSpy.mockRestore(); }); }); + +function createMockDOContext(): ExecutionContext { + return { + waitUntil: vi.fn(), + passThroughOnException: vi.fn(), + storage: {}, + } as unknown as ExecutionContext; +} + +describe('Durable Object (DO) context', () => { + test('DO handler registers teardown via waitUntil', async () => { + const context = createMockDOContext(); + const waitUntilSpy = vi.spyOn(context, 'waitUntil'); + + // Send a body with a content-length so the response is treated as non-streaming + // and teardown runs at the handler boundary rather than on stream completion. + const result = await wrapRequestHandler( + { options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, + () => new Response('test', { headers: { 'content-type': 'application/json' } }), + ); + + expect(result.status).toBe(200); + // Teardown is registered via waitUntil (a DurableObjectState.waitUntil exists + // for API compatibility and still runs the passed promise) + expect(waitUntilSpy).toHaveBeenCalled(); + }); + + test('DO handler error path registers teardown via waitUntil', async () => { + const context = createMockDOContext(); + const waitUntilSpy = vi.spyOn(context, 'waitUntil'); + 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 + } + + // Teardown is registered via waitUntil on error too + expect(waitUntilSpy).toHaveBeenCalled(); + // And flush runs as part of that teardown + expect(flushSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + }); + + test('DO handler for OPTIONS registers teardown via waitUntil', async () => { + const context = createMockDOContext(); + const waitUntilSpy = vi.spyOn(context, 'waitUntil'); + 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 }), + ); + + expect(waitUntilSpy).toHaveBeenCalled(); + expect(flushSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + }); + + test('DO handler for HEAD registers teardown via waitUntil', async () => { + const context = createMockDOContext(); + const waitUntilSpy = vi.spyOn(context, 'waitUntil'); + 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 }), + ); + + expect(waitUntilSpy).toHaveBeenCalled(); + expect(flushSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + }); + + test('DO handler for streaming response registers teardown via waitUntil', async () => { + const context = createMockDOContext(); + const waitUntilSpy = vi.spyOn(context, 'waitUntil'); + 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), + ); + + await result.text(); + + // Teardown is registered via waitUntil + expect(waitUntilSpy).toHaveBeenCalled(); + // And flush runs as part of that teardown + expect(flushSpy).toHaveBeenCalled(); + + flushSpy.mockRestore(); + }); + + test('DO handler for protocol upgrade (101) registers teardown via waitUntil', async () => { + const context = createMockDOContext(); + const waitUntilSpy = vi.spyOn(context, 'waitUntil'); + const flushSpy = vi.spyOn(CloudflareClient.prototype, 'flush').mockResolvedValue(true); + const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose'); + + 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, + ); + + // Teardown is registered via waitUntil + expect(waitUntilSpy).toHaveBeenCalled(); + // Flush runs as part of that teardown + expect(flushSpy).toHaveBeenCalled(); + // Dispose should NOT be called for 101 + expect(disposeSpy).not.toHaveBeenCalled(); + + flushSpy.mockRestore(); + disposeSpy.mockRestore(); + }); +}); + +describe('cached client (cacheClient)', () => { + beforeEach(() => { + _clearGlobalClientCache(); + }); + + // `init()` resolves defaults into the options object it is given, so each call + // needs a fresh object to fingerprint identically — exactly like real callers, + // which build their options per invocation. + const makeOptions = (dsn?: string): CloudflareOptions => ({ + dsn: dsn ?? MOCK_OPTIONS.dsn, + beforeSend() { + return null; + }, + }); + + test('returns the same cached client for the same options', async () => { + const client1 = init(makeOptions()); + const client2 = init(makeOptions()); + expect(client2).toBe(client1); + }); + + test('returns the isolate client even when a later init uses a different DSN', async () => { + const client1 = init(makeOptions()); + const client2 = init(makeOptions('https://other@dsn.ingest.sentry.io/9999')); + expect(client2).toBe(client1); + }); + + test('clears cache with _clearGlobalClientCache', async () => { + const client1 = init(makeOptions()); + _clearGlobalClientCache(); + const client2 = init(makeOptions()); + expect(client2).not.toBe(client1); + }); +}); diff --git a/packages/cloudflare/test/sdk.test.ts b/packages/cloudflare/test/sdk.test.ts index e706e5465fb3..d6c454e06edf 100644 --- a/packages/cloudflare/test/sdk.test.ts +++ b/packages/cloudflare/test/sdk.test.ts @@ -1,7 +1,8 @@ import * as SentryCore from '@sentry/core'; -import type { Integration } from '@sentry/core'; +import type { Envelope, Integration } from '@sentry/core'; import { getClient } from '@sentry/core'; import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import type { CloudflareOptions } from '../src/client'; import { CloudflareClient } from '../src/client'; import { getDefaultIntegrations, init } from '../src/sdk'; import { resetSdk } from './testUtils'; @@ -82,6 +83,210 @@ describe('init', () => { }); }); +describe('cacheClient', () => { + beforeEach(() => { + resetSdk(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const TEST_ENVELOPE = [ + { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '2023-05-31T12:00:00.000Z' }, + [[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }]], + ] as Envelope; + + test('returns the same client for repeated init with identical options', () => { + const options = { + dsn: 'https://public@dsn.ingest.sentry.io/1337', + } as const; + + const first = init({ ...options }); + const second = init({ ...options }); + + expect(second).toBe(first); + }); + + test('returns the isolate client when later init options differ', () => { + const first = init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 0.5, + }); + const second = init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, + }); + + expect(second).toBe(first); + }); + + test('re-binds the cached client to the current scope on repeated init', () => { + const options = { + dsn: 'https://public@dsn.ingest.sentry.io/1337', + } as const; + + const cached = init({ ...options }); + + // Simulate a competing init leaving a different client bound to the scope + SentryCore.getCurrentScope().setClient(undefined); + expect(getClient()).toBeUndefined(); + + const again = init({ ...options }); + expect(again).toBe(cached); + expect(getClient()).toBe(cached); + }); + + test('creates a fresh client when the cached one was disposed', () => { + const options = { + dsn: 'https://public@dsn.ingest.sentry.io/1337', + } as const; + + const cached = init({ ...options }); + cached?.dispose(); + + const again = init({ ...options }); + expect(again).toBeDefined(); + expect(again).not.toBe(cached); + expect(again?.getTransport()).toBeDefined(); + }); + + test('flushes eagerly when an envelope is sent on a cached client', async () => { + // The eager drain fires the buffered fetch, so stub out the network + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('ok'))); + + const client = init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + }); + + const transport = client?.getTransport(); + expect(transport).toBeDefined(); + + const flushSpy = vi.spyOn(transport!, 'flush'); + await client!.sendEnvelope(TEST_ENVELOPE); + + expect(flushSpy).toHaveBeenCalled(); + }); + + test('does not flush eagerly when cacheClient is disabled', async () => { + const client = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', cacheClient: false }); + + const transport = client?.getTransport(); + expect(transport).toBeDefined(); + + const flushSpy = vi.spyOn(transport!, 'flush'); + await client!.sendEnvelope(TEST_ENVELOPE); + + expect(flushSpy).not.toHaveBeenCalled(); + }); + + // Logs and metrics batch client-side and the idle drain timer is disabled for this + // runtime, so unlike an event a capture alone never produces an envelope. A cached + // client never reaches an invocation-boundary flush, so without an eager drain these + // are dropped entirely — and silently, since errors keep working. + describe('log and metric delivery', () => { + function initWithCapturingTransport(options: Partial = {}) { + const envelopes: Envelope[] = []; + const client = init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + enableLogs: true, + transport: () => ({ + send: (envelope: Envelope) => { + envelopes.push(envelope); + return Promise.resolve({}); + }, + flush: () => Promise.resolve(true), + }), + ...options, + })!; + + return { client, envelopes }; + } + + const itemTypes = (envelopes: Envelope[]): string[] => + envelopes.map(envelope => (envelope[1]?.[0]?.[0] as { type: string })?.type); + + test('delivers a log captured on a cached client without an explicit flush', async () => { + const { envelopes } = initWithCapturingTransport(); + + SentryCore.logger.info('detached log'); + await vi.waitFor(() => expect(itemTypes(envelopes)).toContain('log')); + }); + + test('delivers a metric captured on a cached client without an explicit flush', async () => { + const { envelopes } = initWithCapturingTransport(); + + SentryCore.metrics.count('detached_metric', 1); + await vi.waitFor(() => expect(itemTypes(envelopes)).toContain('trace_metric')); + }); + + test('coalesces a synchronous burst of logs into a single envelope', async () => { + const { envelopes } = initWithCapturingTransport(); + + for (let i = 0; i < 5; i++) { + SentryCore.logger.info(`burst ${i}`); + } + + await vi.waitFor(() => expect(itemTypes(envelopes)).toContain('log')); + expect(itemTypes(envelopes).filter(type => type === 'log')).toHaveLength(1); + }); + + test('keeps batching logs until flush for a non-cached client', async () => { + const { client, envelopes } = initWithCapturingTransport({ cacheClient: false }); + + SentryCore.logger.info('batched log'); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(envelopes).toHaveLength(0); + + await client.flush(0); + expect(itemTypes(envelopes)).toContain('log'); + }); + + test('flush() delivers buffered logs on a cached client', async () => { + const { client, envelopes } = initWithCapturingTransport(); + + SentryCore.logger.info('tail log'); + await client.flush(0); + + expect(itemTypes(envelopes)).toContain('log'); + }); + }); + + test('applies initialScope on every cached init, not just the first', () => { + const options = { + dsn: 'https://public@dsn.ingest.sentry.io/1337', + } as const; + + init({ ...options }); + SentryCore.getCurrentScope().clear(); + + init({ ...options, initialScope: { tags: { from: 'initialScope' } } }); + + expect(SentryCore.getCurrentScope().getScopeData().tags).toEqual({ from: 'initialScope' }); + }); + + test('does not instrument ctx.waitUntil with the flush lock for cached clients', () => { + const waitUntil = vi.fn(); + const context = { waitUntil, passThroughOnException: vi.fn() }; + + init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + ctx: context, + }); + + expect(context.waitUntil).toBe(waitUntil); + }); + + test('instruments ctx.waitUntil with the flush lock for non-cached clients', () => { + const waitUntil = vi.fn(); + const context = { waitUntil, passThroughOnException: vi.fn() }; + + init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', cacheClient: false, ctx: context }); + + expect(context.waitUntil).not.toBe(waitUntil); + }); +}); + describe('getDefaultIntegrations', () => { afterEach(() => { delete globalThis.__SENTRY_ORCHESTRION__; diff --git a/packages/cloudflare/test/testUtils.ts b/packages/cloudflare/test/testUtils.ts index 818daec6ad23..79bdfa403365 100644 --- a/packages/cloudflare/test/testUtils.ts +++ b/packages/cloudflare/test/testUtils.ts @@ -1,5 +1,6 @@ import { context, propagation, trace } from '@opentelemetry/api'; import { getMainCarrier } from '@sentry/core'; +import { _clearGlobalClientCache } from '../src/clientCache'; function resetGlobals(): void { getMainCarrier().__SENTRY__ = undefined; @@ -15,4 +16,5 @@ function cleanupOtel(): void { export function resetSdk(): void { resetGlobals(); cleanupOtel(); + _clearGlobalClientCache(); } diff --git a/packages/cloudflare/test/utils/invocationContext.test.ts b/packages/cloudflare/test/utils/invocationContext.test.ts new file mode 100644 index 000000000000..d560b21e9707 --- /dev/null +++ b/packages/cloudflare/test/utils/invocationContext.test.ts @@ -0,0 +1,76 @@ +import { getDefaultIsolationScope, getIsolationScope, GLOBAL_OBJ, withIsolationScope } from '@sentry/core'; +import { AsyncLocalStorage } from 'async_hooks'; +import { setAsyncLocalStorageAsyncContextStrategy } from '@sentry/server-utils/no-diagnostic-channels'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { getInvocationState, setInvocationState } from '../../src/utils/invocationContext'; +import { withInvocationIsolationScope } from '../../src/utils/invocationScope'; + +describe('invocation state', () => { + beforeEach(() => { + (GLOBAL_OBJ as never).AsyncLocalStorage = AsyncLocalStorage; + setAsyncLocalStorageAsyncContextStrategy(); + }); + + it('returns undefined outside any invocation', () => { + expect(getInvocationState()).toBeUndefined(); + }); + + it('returns undefined for a forked scope that carries no state', () => { + withIsolationScope(getDefaultIsolationScope().clone(), () => { + expect(getInvocationState()).toBeUndefined(); + }); + }); + + it('exposes state attached to the active isolation scope', () => { + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const scope = getDefaultIsolationScope().clone(); + setInvocationState(scope, { ctx }); + + withIsolationScope(scope, () => { + expect(getInvocationState()?.ctx).toBe(ctx); + }); + + expect(getInvocationState()).toBeUndefined(); + }); + + it('is not inherited by scope clones', () => { + const scope = getDefaultIsolationScope().clone(); + setInvocationState(scope, { ctx: undefined }); + + withIsolationScope(scope.clone(), () => { + expect(getInvocationState()).toBeUndefined(); + }); + }); + + it('is attached at the invocation entry point and kept when reentrant', () => { + const outerCtx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const innerCtx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + withInvocationIsolationScope(() => { + expect(getInvocationState()?.ctx).toBe(outerCtx); + + withInvocationIsolationScope(() => { + expect(getInvocationState()?.ctx).toBe(outerCtx); + }, innerCtx); + }, outerCtx); + }); + + it('isolates state between concurrent invocations', async () => { + const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + await Promise.all([ + withInvocationIsolationScope(async () => { + await new Promise(resolve => setTimeout(resolve, 10)); + expect(getInvocationState()?.ctx).toBe(ctxA); + }, ctxA), + withInvocationIsolationScope(async () => { + await new Promise(resolve => setTimeout(resolve, 5)); + expect(getInvocationState()?.ctx).toBe(ctxB); + }, ctxB), + ]); + + expect(getIsolationScope()).toBe(getDefaultIsolationScope()); + expect(getInvocationState()).toBeUndefined(); + }); +}); diff --git a/packages/cloudflare/test/workflow.test.ts b/packages/cloudflare/test/workflow.test.ts index 2578c1e0343d..0e36e15ab5d7 100644 --- a/packages/cloudflare/test/workflow.test.ts +++ b/packages/cloudflare/test/workflow.test.ts @@ -3,6 +3,7 @@ import { startSpan } from '@sentry/core'; import type { WorkflowEvent, WorkflowStep, WorkflowStepConfig } from 'cloudflare:workers'; import { beforeEach, describe, expect, test, vi } from 'vitest'; import { deterministicTraceIdFromInstanceId, instrumentWorkflowWithSentry } from '../src/workflows'; +import { resetSdk } from './testUtils'; vi.mock('../src/instrumentations/worker/instrumentEnv', () => ({ instrumentEnv: vi.fn((env: unknown) => env), @@ -104,6 +105,7 @@ async function drainWaitUntilLikeCloudflareVitestPool( describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { beforeEach(() => { + resetSdk(); vi.clearAllMocks(); }); @@ -133,8 +135,10 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('first step', expect.any(Function)); - // We flush after the step.do and at the end of the run - expect(mockContext.waitUntil).toHaveBeenCalledTimes(2); + // We flush after the step.do and at the end of the run, plus one + // waitUntil registration for the eagerly delivered envelope + // and one for the envelope send itself + expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); expect(mockTransport.send).toHaveBeenCalledTimes(1); expect(mockTransport.send).toHaveBeenCalledWith([ @@ -379,8 +383,10 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('first step', expect.any(Function)); - // We flush after the step.do and at the end of the run - expect(mockContext.waitUntil).toHaveBeenCalledTimes(2); + // We flush after the step.do and at the end of the run, plus one + // waitUntil registration for the eagerly delivered envelope + // and one for the envelope send itself + expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); expect(mockTransport.send).toHaveBeenCalledTimes(1); expect(mockTransport.send).toHaveBeenCalledWith([ @@ -453,8 +459,10 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('sometimes error step', expect.any(Function)); - // One flush for the failed attempt, one for the retry success, one at end of run - expect(mockContext.waitUntil).toHaveBeenCalledTimes(3); + // One flush for the failed attempt, one for the retry success, one at end of run, + // plus one waitUntil registration per eagerly delivered envelope + // and one per envelope send + expect(mockContext.waitUntil).toHaveBeenCalledTimes(7); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); // No error event (not final attempt), only failed transaction + successful retry transaction expect(mockTransport.send).toHaveBeenCalledTimes(2); @@ -724,8 +732,10 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { const event = { payload: {}, timestamp: new Date(), instanceId: INSTANCE_ID }; await workflow.run(event, mockStep); - // Flush after step.do and at end of run - expect(mockContext.waitUntil).toHaveBeenCalledTimes(2); + // Flush after step.do and at end of run, plus one + // waitUntil registration for the eagerly delivered envelope + // and one for the envelope send itself + expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); expect(mockTransport.send).toHaveBeenCalledTimes(1); const sendArg = mockTransport.send.mock.calls[0]![0]; From 083429442401d0cc7fb9d7b3618e357142aa8c6b Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 7 Aug 2026 15:43:39 +0200 Subject: [PATCH 02/14] fixup! feat(cloudflare): Add cacheClient to reuse the client across invocations --- .../cloudflare-integration-tests/runner.ts | 15 ++++++- .../suites/cache-client/test.ts | 18 ++++---- packages/cloudflare/src/baseSdk.ts | 30 ++++++++++++- packages/cloudflare/src/sdk.ts | 43 +------------------ packages/cloudflare/test/request.test.ts | 21 +++++++++ 5 files changed, 74 insertions(+), 53 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/runner.ts b/dev-packages/cloudflare-integration-tests/runner.ts index 40e864d0a817..19873330d82e 100644 --- a/dev-packages/cloudflare-integration-tests/runner.ts +++ b/dev-packages/cloudflare-integration-tests/runner.ts @@ -161,6 +161,7 @@ export function createRunner(...paths: string[]) { // controls whether envelopes are expected in predefined order or not let unordered = false; + let failOnUnexpected = false; if (!existsSync(testPath)) { throw new Error(`Test scenario not found: ${testPath}`); @@ -195,6 +196,10 @@ export function createRunner(...paths: string[]) { unordered = true; return this; }, + failOnUnexpected: function () { + failOnUnexpected = true; + return this; + }, ignore: function (...types: EnvelopeItemType[]) { types.forEach(t => ignored.add(t)); return this; @@ -222,6 +227,7 @@ export function createRunner(...paths: string[]) { const expectedEnvelopeCount = expectedEnvelopes.length; let envelopeCount = 0; + let unexpectedEnvelopeError: Error | undefined; const envelopeWaiters: { expected: Expected; resolve: () => void; reject: (e: unknown) => void }[] = []; const { resolve: setWorkerPort, @@ -297,6 +303,10 @@ export function createRunner(...paths: string[]) { // no match found if (matchIndex < 0) { + if (failOnUnexpected) { + unexpectedEnvelopeError ??= new Error('Received an unexpected envelope'); + reject(unexpectedEnvelopeError); + } return; } @@ -429,7 +439,10 @@ export function createRunner(...paths: string[]) { return { completed: async function (): Promise { - return isComplete; + await isComplete; + if (unexpectedEnvelopeError) { + throw unexpectedEnvelopeError; + } }, makeRequest: async function ( method: 'get' | 'post', diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts index dde6864391c6..38fcdce95913 100644 --- a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts @@ -134,17 +134,19 @@ it('cacheClient: false - repro #22545: detached work events are silently dropped it('cacheClient: true - dedupe drops the same error across invocations', async ({ signal }) => { // A shared client shares its dedupe state, so the same error captured by two separate // invocations is reported only once — the second is dropped as a duplicate. - const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); - - await runner.makeRequestAndWaitForEnvelope( - 'get', - '/cache/dedupe?id=dedupe-shared', - errorEventExpectation('Same error', CAPTURE_MECHANISM), - ); + const runner = createRunner(__dirname) + .ignore('transaction', 'span') + .unordered() + .failOnUnexpected() + .expect(errorEventExpectation('Same error', CAPTURE_MECHANISM)) + .start(signal); - // Second and third invocations capture the same error, but dedupe drops them. + // All three invocations are made without per-request waiters, while the runner requires the + // single expected error and rejects if either duplicate is delivered unexpectedly. + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); + await runner.completed(); }); it('cacheClient: false - dedupe does not persist across invocations', async ({ signal }) => { diff --git a/packages/cloudflare/src/baseSdk.ts b/packages/cloudflare/src/baseSdk.ts index cdea4dc5775b..c6fd00c1d670 100644 --- a/packages/cloudflare/src/baseSdk.ts +++ b/packages/cloudflare/src/baseSdk.ts @@ -1,4 +1,5 @@ import type { Integration } from '@sentry/core'; +import { getCurrentScope, setCurrentClient } from '@sentry/core'; import { consoleIntegration, conversationIdIntegration, @@ -15,6 +16,7 @@ import { import type { CloudflareClientOptions, CloudflareOptions } from './client'; import { CloudflareClient } from './client'; import { makeFlushLock } from './flush'; +import { cacheClient, getCachedClient } from './clientCache'; import { fetchIntegration } from './integrations/fetch'; import { httpServerIntegration } from './integrations/httpServer'; import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration } from './integrations/spotlight'; @@ -77,12 +79,31 @@ export function getBaseDefaultIntegrations(options: CloudflareOptions): Integrat * Node.js-only code. `request.ts` — which backs both `wrapRequestHandler` and the * `@sentry/cloudflare/request` entry point, and therefore has to work on runtimes without the * `nodejs_compat` compatibility flag — creates its client from here instead of from `sdk.ts`. + * + * The client is cached and reused across invocations within the same isolate, + * unless `cacheClient: false` is passed. This avoids the + * per-invocation cost of constructing a new client, and it is what makes + * Durable Object telemetry reliable: a per-invocation client is disposed at + * the end of the handler, and in a Durable Object there is no `waitUntil` + * boundary that reliably extends execution, so spans/events that end after + * disposal would otherwise be lost. */ export function initWithDefaultIntegrations( options: CloudflareOptions, getDefaultIntegrationsImpl: (options: CloudflareOptions) => Integration[], - { skipFlushLock = false }: { skipFlushLock?: boolean } = {}, ): CloudflareClient | undefined { + const cacheEnabled = options.cacheClient !== false && Boolean(options.dsn); + + if (cacheEnabled) { + const cached = getCachedClient(); + if (cached?.getTransport()) { + getCurrentScope().update(options.initialScope); + setCurrentClient(cached); + cached.setExecutionContext(options.ctx); + return cached; + } + } + if (options.defaultIntegrations === undefined) { options.defaultIntegrations = getDefaultIntegrationsImpl(options); } @@ -91,11 +112,12 @@ export function initWithDefaultIntegrations( // invocation's flush lock would make later flushes wait on that invocation's // waitUntil work forever. Eager delivery replaces the flush lock's purpose. const invocationContext = options.ctx; - const flushLock = !skipFlushLock && invocationContext ? makeFlushLock(invocationContext) : undefined; + const flushLock = !cacheEnabled && invocationContext ? makeFlushLock(invocationContext) : undefined; delete options.ctx; const clientOptions: CloudflareClientOptions = { ...options, + cacheClient: cacheEnabled, stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser), integrations: getIntegrationsToSetup(options), transport: options.transport || makeCloudflareTransport, @@ -124,6 +146,10 @@ export function initWithDefaultIntegrations( const client = initAndBind(CloudflareClient, clientOptions) as CloudflareClient; + if (cacheEnabled && client && options.dsn) { + cacheClient(client); + } + // An instrumented module that first evaluates AFTER this init (e.g. a driver // lazily required on first use) stores its subscriber factory on the global // marker too late for the default-integrations snapshot above. Its injected diff --git a/packages/cloudflare/src/sdk.ts b/packages/cloudflare/src/sdk.ts index d940e58c8845..e786a6920779 100644 --- a/packages/cloudflare/src/sdk.ts +++ b/packages/cloudflare/src/sdk.ts @@ -1,8 +1,6 @@ import type { Integration } from '@sentry/core'; -import { getCurrentScope, setCurrentClient } from '@sentry/core'; import { getBaseDefaultIntegrations, initWithDefaultIntegrations } from './baseSdk'; import type { CloudflareClient, CloudflareOptions } from './client'; -import { cacheClient, getCachedClient } from './clientCache'; // Test-only helper, re-exported here so tests can reset the global client cache. export { _clearGlobalClientCache } from './clientCache'; @@ -16,46 +14,7 @@ export function getDefaultIntegrations(options: CloudflareOptions): Integration[ /** * Initializes the cloudflare SDK. - * - * The client is cached and reused across invocations within the same isolate, - * unless `cacheClient: false` is passed. This avoids the - * per-invocation cost of constructing a new client, and it is what makes - * Durable Object telemetry reliable: a per-invocation client is disposed at - * the end of the handler, and in a Durable Object there is no `waitUntil` - * boundary that reliably extends execution, so spans/events that end after - * disposal would otherwise be lost. */ export function init(options: CloudflareOptions): CloudflareClient | undefined { - const cacheEnabled = options.cacheClient !== false; - - if (cacheEnabled) { - // Normalize the flag so the client marks itself as cached. - options.cacheClient = true; - } - - if (cacheEnabled && options.dsn) { - const cached = getCachedClient(); - // A cached client that has lost its transport was disposed. Replace it rather - // than returning a dead client for the rest of the isolate's lifetime. - if (cached?.getTransport()) { - // Mirror the two scope side effects of `initAndBind`, which only runs on first - // creation. Without the re-bind the scope keeps whatever client a previous init - // left behind — which may have been disposed since — and without the update - // `initialScope` would apply only to an isolate's very first invocation. - getCurrentScope().update(options.initialScope); - setCurrentClient(cached); - // The cached client outlives the invocation that created it, so its eager - // sends must be registered with the current invocation's waitUntil. - cached.setExecutionContext(options.ctx); - return cached; - } - } - - const client = initWithDefaultIntegrations(options, getDefaultIntegrations, { skipFlushLock: cacheEnabled }); - - if (cacheEnabled && client && options.dsn) { - cacheClient(client); - } - - return client; + return initWithDefaultIntegrations(options, getDefaultIntegrations); } diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index 4c9ed540397e..f6a46d23b342 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -15,6 +15,7 @@ import { _clearGlobalClientCache, init } from '../src/sdk'; const MOCK_OPTIONS: CloudflareOptions = { dsn: 'https://public@dsn.ingest.sentry.io/1337', traceLifecycle: 'static', + cacheClient: false, }; const NODE_MAJOR_VERSION = parseInt(process.versions.node.split('.')[0]!); @@ -1151,6 +1152,26 @@ describe('cached client (cacheClient)', () => { }, }); + test('wrapRequestHandler reuses a client when cacheClient is enabled', async () => { + const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind'); + const options = { ...MOCK_OPTIONS, cacheClient: true }; + + await wrapRequestHandler( + { options, request: new Request('https://example.com/first'), context: createMockExecutionContext() }, + () => new Response('first'), + ); + await wrapRequestHandler( + { + options: { ...options }, + request: new Request('https://example.com/second'), + context: createMockExecutionContext(), + }, + () => new Response('second'), + ); + + expect(initAndBindSpy).toHaveBeenCalledTimes(1); + }); + test('returns the same cached client for the same options', async () => { const client1 = init(makeOptions()); const client2 = init(makeOptions()); From e52d07345da34b856cfd5f919eaa3a1a9c3383ea Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 7 Aug 2026 15:50:51 +0200 Subject: [PATCH 03/14] chore: Update size-limit --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index becbd8285041..c158c70f67b7 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -485,7 +485,7 @@ module.exports = [ ignore: [...builtinModules, ...nodePrefixedBuiltinModules], gzip: false, brotli: false, - limit: '522 KiB', + limit: '540 KiB', disablePlugins: ['@size-limit/webpack'], webpack: false, modifyEsbuildConfig: function (config) { From 2cce5567ba31979fc96e8022d4f483cbb24ea832 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 10 Aug 2026 12:42:07 +0200 Subject: [PATCH 04/14] fixup! feat(cloudflare): Add cacheClient to reuse the client across invocations --- .../suites/cache-client/index.ts | 13 +++++++------ .../suites/cache-client/test.ts | 13 ++++++++++--- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts index 1ef5f389fcd3..fab57966b979 100644 --- a/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/index.ts @@ -47,8 +47,8 @@ class CacheDurableObjectBase extends DurableObject { throw new Error(`Cache DO handler error from ${instanceId}`); } - async dedupe(): Promise { - Sentry.captureException(new Error('Same error')); + async dedupe(errorMessage: string): Promise { + Sentry.captureException(new Error(errorMessage)); return 'dedupe test'; } @@ -91,8 +91,8 @@ class NoCacheDurableObjectBase extends DurableObject { throw new Error(`No-cache DO handler error from ${instanceId}`); } - async dedupe(): Promise { - Sentry.captureException(new Error('Same error')); + async dedupe(errorMessage: string): Promise { + Sentry.captureException(new Error(errorMessage)); return 'dedupe test'; } @@ -141,6 +141,7 @@ export default Sentry.withSentry( async fetch(request, env, ctx) { const url = new URL(request.url); const instanceId = url.searchParams.get('id') || 'default'; + const errorMessage = url.searchParams.get('errorMessage') || 'Same error'; // Work that finishes AFTER the response: a post-response span tree plus a // log, metric and error, all registered via waitUntil. This is the worker-side @@ -187,7 +188,7 @@ export default Sentry.withSentry( const stub = env.CACHE_DO.get( env.CACHE_DO.idFromName(`cache-do-${instanceId}`), ) as DurableObjectStub; - const result = await stub.dedupe(); + const result = await stub.dedupe(errorMessage); return new Response(String(result)); } @@ -233,7 +234,7 @@ export default Sentry.withSentry( const stub = env.NO_CACHE_DO.get( env.NO_CACHE_DO.idFromName(`no-cache-do-${instanceId}`), ) as DurableObjectStub; - const result = await stub.dedupe(); + const result = await stub.dedupe(errorMessage); return new Response(String(result)); } diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts index 38fcdce95913..52bfe087cb2c 100644 --- a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts @@ -139,13 +139,20 @@ it('cacheClient: true - dedupe drops the same error across invocations', async ( .unordered() .failOnUnexpected() .expect(errorEventExpectation('Same error', CAPTURE_MECHANISM)) + .expect(errorEventExpectation('Another error', CAPTURE_MECHANISM)) + .expect(errorEventExpectation('Same error', CAPTURE_MECHANISM)) .start(signal); // All three invocations are made without per-request waiters, while the runner requires the // single expected error and rejects if either duplicate is delivered unexpectedly. - await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); - await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); - await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Another error'); + await runner.makeRequest('get', '/cache/dedupe?id=dedupe-shared&errorMessage=Same error'); await runner.completed(); }); From 94ba10903168b6ede15bcb381c9e53b8a694e1e1 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 10 Aug 2026 12:44:09 +0200 Subject: [PATCH 05/14] fixup! feat(cloudflare): Add cacheClient to reuse the client across invocations --- .../suites/cache-client/test.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts index 52bfe087cb2c..12f798b61ae7 100644 --- a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts @@ -257,19 +257,6 @@ describe('durable object storage spans', () => { } }); -it('cacheClient: true - multiple DO instances share the same client', async ({ signal }) => { - const runner = createRunner(__dirname) - .expect(errorEventExpectation('Cache DO handler error from instance-1', DO_MECHANISM)) - .expect(errorEventExpectation('Cache DO handler error from instance-2', DO_MECHANISM)) - .unordered() - .start(signal); - - // Two different DO instances — both should capture errors - await runner.makeRequest('get', '/cache/handler-error?id=instance-1', { expectError: true }); - await runner.makeRequest('get', '/cache/handler-error?id=instance-2', { expectError: true }); - await runner.completed(); -}); - // The worker-side half of #22545: work registered via ctx.waitUntil finishes after // the response and after the invocation's flush point, so the spans/log/metric/error // are only delivered because the cached client drains them eagerly. From 2d9e820a17529c2b6e852c567cef6f9e651fa854 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 10 Aug 2026 16:41:58 +0200 Subject: [PATCH 06/14] test(cloudflare): Do not cache client for legacy workflows --- .../cloudflare-workers-workflow-legacy/src/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers-workflow-legacy/src/index.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers-workflow-legacy/src/index.ts index cd99841103dc..2147fa3a69eb 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers-workflow-legacy/src/index.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers-workflow-legacy/src/index.ts @@ -50,6 +50,9 @@ export default Sentry.withSentry( traceLifecycle: 'static', dsn: env.E2E_TEST_DSN, tunnel: 'http://localhost:3031/', + // Do not cache this client, as locally there is only one instance + // And when this handler is getting cached, then the workflow will reuse this very client + cacheClient: false, }), { async fetch(request, env, _ctx) { From 165c105231965fc294feca588abdca63ae7f28c0 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Tue, 11 Aug 2026 09:33:27 +0200 Subject: [PATCH 07/14] fix(cloudflare): Flush per traceId and store them --- packages/cloudflare/src/client.ts | 10 ++++-- .../cloudflare/src/utils/invocationContext.ts | 2 +- packages/cloudflare/test/client.test.ts | 34 +++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 2ba0564ef76d..614eb948906a 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -288,10 +288,14 @@ export class CloudflareClient extends ServerRuntimeClient { // ending before it batch in the buffer and are drained by the boundary // flush. RPC sub-invocations in Durable Objects never reach a flush point, // so their spans batch one envelope per trace here. - if (!invocationState?.flushPointReached || invocationState.spanFlushScheduled) { + if (!invocationState?.flushPointReached) { return; } - invocationState.spanFlushScheduled = true; + const pendingTraceIds = (invocationState.pendingSpanFlushTraceIds ??= new Set()); + if (pendingTraceIds.has(span.spanContext().traceId)) { + return; + } + pendingTraceIds.add(span.spanContext().traceId); // The trace id must come from the span, not the current scope: `continueTrace` // writes the propagation context to the *current* scope, so the forked // isolation scope's propagation context carries a different trace id and @@ -304,7 +308,7 @@ export class CloudflareClient extends ServerRuntimeClient { // runs in the same async context, so the send stays attributed to this // invocation. queueMicrotask(() => { - invocationState.spanFlushScheduled = false; + pendingTraceIds.delete(traceId); this.emit('flushTraceSpans', traceId); }); }); diff --git a/packages/cloudflare/src/utils/invocationContext.ts b/packages/cloudflare/src/utils/invocationContext.ts index ec1f450bb885..7504b461224b 100644 --- a/packages/cloudflare/src/utils/invocationContext.ts +++ b/packages/cloudflare/src/utils/invocationContext.ts @@ -37,7 +37,7 @@ export interface InvocationState { * their flush point each schedule their own flush in their own async context — * the flush and its envelope send stay attributed to the owning invocation. */ - spanFlushScheduled?: boolean; + pendingSpanFlushTraceIds?: Set; /** Eager transport drains owned by this invocation, serialized in capture order. */ eagerFlushPromise?: PromiseLike; } diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 3bb3c542453c..9687ec6140e3 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -770,6 +770,40 @@ describe('CloudflareClient', () => { expect(flushSpy).toHaveBeenCalledWith('trace-b'); }); + it('flushes every trace when one invocation ends spans of several traces in the same turn', async () => { + const { client, flushSpy } = makeCachedClient(); + + // A single invocation can own more than one trace past its flush point — + // e.g. two `startNewTrace` background jobs completing synchronously inside + // one `ctx.waitUntil`. Debouncing per invocation instead of per trace would + // schedule only the first trace and leave the rest buffered with no later + // in-invocation drain. + await withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-a') as never); + client.emit('afterSpanEnd', makeSpan('trace-b') as never); + await tick(); + }, ctx as never); + + expect(flushSpy).toHaveBeenCalledTimes(2); + expect(flushSpy).toHaveBeenCalledWith('trace-a'); + expect(flushSpy).toHaveBeenCalledWith('trace-b'); + }); + + it('still collapses repeated span ends of the same trace into one flush', async () => { + const { client, flushSpy } = makeCachedClient(); + + await withInvocationIsolationScope(async () => { + await client.flush(0); + client.emit('afterSpanEnd', makeSpan('trace-a') as never); + client.emit('afterSpanEnd', makeSpan('trace-a') as never); + await tick(); + }, ctx as never); + + expect(flushSpy).toHaveBeenCalledTimes(1); + expect(flushSpy).toHaveBeenCalledWith('trace-a'); + }); + it("flushes each invocation's trace in its own async context", async () => { const { client } = makeCachedClient(); const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; From ae36488b5e2ed64effde27d00938266ae844c1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Peer=20St=C3=B6cklmair?= Date: Wed, 12 Aug 2026 12:08:33 +0200 Subject: [PATCH 08/14] Update packages/cloudflare/src/client.ts Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com> --- packages/cloudflare/src/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 614eb948906a..eb7caa5074d9 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -299,7 +299,7 @@ export class CloudflareClient extends ServerRuntimeClient { // The trace id must come from the span, not the current scope: `continueTrace` // writes the propagation context to the *current* scope, so the forked // isolation scope's propagation context carries a different trace id and - // flushing by it silently no-ops (measured: ~40% of post-flush traces lost). + // flushing by it silently no-ops. const traceId = span.spanContext().traceId; // Defer to a microtask: a synchronous flush here runs before the span // streaming integration's own `afterSpanEnd` handler has added the From 3e278b53d19721d0279a76123c18194727e21a4d Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 12 Aug 2026 13:44:27 +0200 Subject: [PATCH 09/14] ref: Review changes --- packages/cloudflare/src/baseSdk.ts | 2 +- packages/cloudflare/src/client.ts | 2 +- packages/cloudflare/src/transport.ts | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cloudflare/src/baseSdk.ts b/packages/cloudflare/src/baseSdk.ts index c6fd00c1d670..72b0064a7a78 100644 --- a/packages/cloudflare/src/baseSdk.ts +++ b/packages/cloudflare/src/baseSdk.ts @@ -92,7 +92,7 @@ export function initWithDefaultIntegrations( options: CloudflareOptions, getDefaultIntegrationsImpl: (options: CloudflareOptions) => Integration[], ): CloudflareClient | undefined { - const cacheEnabled = options.cacheClient !== false && Boolean(options.dsn); + const cacheEnabled = options.cacheClient !== false; if (cacheEnabled) { const cached = getCachedClient(); diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index eb7caa5074d9..68210d6c7bf2 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -113,7 +113,7 @@ export class CloudflareClient extends ServerRuntimeClient { // If no more pending spans, resolve the completion promise if (this._pendingSpans.size === 0 && this._resolveSpanCompletion) { - DEBUG_BUILD && debug.log('[CloudflareClient] All spans completed, resolving promise'); + DEBUG_BUILD && debug.log('[CloudflareClient] All spans completed, preparing to flush'); this._resolveSpanCompletion(); this._resetSpanCompletionPromise(); } diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts index 685cb7fbccd5..25d9e05572b9 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -8,6 +8,12 @@ export interface CloudflareTransportOptions extends BaseTransportOptions { fetchOptions?: RequestInit; } +/** + * How many payloads the buffer holds before it starts rejecting new ones. + * + * With `cacheClient` a single client is reused across an isolate's invocations, so one buffer now has to absorb + * the payloads of many invocations rather than one. 256 is the size that held up under load testing that scenario. + */ const DEFAULT_TRANSPORT_BUFFER_SIZE = 256; /** From a5f23a9f99a4e29ed82fc06bddb203610cd23b70 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 12 Aug 2026 14:07:41 +0200 Subject: [PATCH 10/14] ref: Remove double init - as there is only 1 client each isolate --- packages/cloudflare/test/sdk.test.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/cloudflare/test/sdk.test.ts b/packages/cloudflare/test/sdk.test.ts index d6c454e06edf..1146b5eb8bc7 100644 --- a/packages/cloudflare/test/sdk.test.ts +++ b/packages/cloudflare/test/sdk.test.ts @@ -252,19 +252,6 @@ describe('cacheClient', () => { }); }); - test('applies initialScope on every cached init, not just the first', () => { - const options = { - dsn: 'https://public@dsn.ingest.sentry.io/1337', - } as const; - - init({ ...options }); - SentryCore.getCurrentScope().clear(); - - init({ ...options, initialScope: { tags: { from: 'initialScope' } } }); - - expect(SentryCore.getCurrentScope().getScopeData().tags).toEqual({ from: 'initialScope' }); - }); - test('does not instrument ctx.waitUntil with the flush lock for cached clients', () => { const waitUntil = vi.fn(); const context = { waitUntil, passThroughOnException: vi.fn() }; From 1b38a6993c0f60f6eeb62d82396bd0832defba5f Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Thu, 13 Aug 2026 13:15:23 +0200 Subject: [PATCH 11/14] ref(cloudflare): Guard invocation state to be bound only to scope --- packages/cloudflare/src/utils/invocationContext.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cloudflare/src/utils/invocationContext.ts b/packages/cloudflare/src/utils/invocationContext.ts index 7504b461224b..87cdce70c089 100644 --- a/packages/cloudflare/src/utils/invocationContext.ts +++ b/packages/cloudflare/src/utils/invocationContext.ts @@ -54,6 +54,10 @@ type ScopeWithInvocationState = Scope & { * scope, which is shared by every invocation in the isolate. */ export function setInvocationState(scope: Scope, state: InvocationState): void { + if (scope === getDefaultIsolationScope()) { + return; + } + (scope as ScopeWithInvocationState)[INVOCATION_STATE] = state; } From b34adc9a6e10ae29024ad06e707dc526a21aee70 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Sun, 23 Aug 2026 14:37:59 +0300 Subject: [PATCH 12/14] ref: Remove DSN check in baseSdk --- packages/cloudflare/src/baseSdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cloudflare/src/baseSdk.ts b/packages/cloudflare/src/baseSdk.ts index 72b0064a7a78..dfbf1ace038b 100644 --- a/packages/cloudflare/src/baseSdk.ts +++ b/packages/cloudflare/src/baseSdk.ts @@ -146,7 +146,7 @@ export function initWithDefaultIntegrations( const client = initAndBind(CloudflareClient, clientOptions) as CloudflareClient; - if (cacheEnabled && client && options.dsn) { + if (cacheEnabled && client) { cacheClient(client); } From 8b39b70fdad8b4ed8bddba8de0ba210d29fc370d Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 24 Aug 2026 15:57:21 +0300 Subject: [PATCH 13/14] ref: review suggestions --- .../suites/cache-client/test.ts | 41 ++- packages/cloudflare/src/baseSdk.ts | 20 +- packages/cloudflare/src/client.ts | 247 +++++---------- packages/cloudflare/src/request.ts | 16 +- .../cloudflare/src/utils/invocationContext.ts | 47 ++- packages/cloudflare/src/workflows.ts | 87 +++-- .../cloudflare/src/wrapMethodWithSentry.ts | 80 +++-- packages/cloudflare/test/client.test.ts | 299 ++++++------------ .../cloudflare/test/durableobject.test.ts | 7 +- .../instrumentWorkerEntrypoint.test.ts | 3 +- packages/cloudflare/test/sdk.test.ts | 45 ++- .../test/utils/invocationContext.test.ts | 61 +++- packages/cloudflare/test/workflow.test.ts | 64 +++- 13 files changed, 520 insertions(+), 497 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts index 12f798b61ae7..77ae00504c4d 100644 --- a/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/cache-client/test.ts @@ -283,21 +283,35 @@ it('cacheClient: true - post-response waitUntil work delivers spans, log, metric // One request fans out into N sequential DO RPC calls. Each RPC span must be // delivered and must belong to the worker request's trace (RPC trace propagation). -it('cacheClient: true - burst DO RPC span shares the worker request trace', async ({ signal }) => { +it('cacheClient: true - burst DO RPC spans share the worker request trace', async ({ signal }) => { let workerTraceId: string | undefined; + let echoSpanCount = 0; const echoTraceIds = new Set(); + // Every RPC call is its own invocation with its own boundary flush, so each echo span + // arrives in its own envelope; the invariant is that all of them carry the worker trace. + const echoEnvelope = (envelope: Envelope): void => { + const payload = envelope[1]?.[0]?.[1] as SpanV2Payload; + const echoSpans = (payload.items ?? []).filter(span => span.name === 'echo'); + + expect(echoSpans.length).toBeGreaterThan(0); + + for (const span of echoSpans) { + expect(span.attributes?.['sentry.op']?.value).toBe('rpc'); + expect(span.trace_id).toBeDefined(); + echoTraceIds.add(span.trace_id!); + echoSpanCount++; + } + }; + const runner = createRunner(__dirname) - .expect((envelope: Envelope) => { - const payload = envelope[1]?.[0]?.[1] as SpanV2Payload; - const echoSpans = (payload.items ?? []).filter(span => span.name === 'echo'); - expect(echoSpans.length).toBeGreaterThan(0); - for (const span of echoSpans) { - expect(span.attributes?.['sentry.op']?.value).toBe('rpc'); - expect(span.trace_id).toBeDefined(); - echoTraceIds.add(span.trace_id!); - } - }) + .expect(echoEnvelope) + .expect(echoEnvelope) + .expect(echoEnvelope) + .expect(echoEnvelope) + .expect(echoEnvelope); + + const started = runner .expect((envelope: Envelope) => { const payload = envelope[1]?.[0]?.[1] as SpanV2Payload; const root = payload.items?.find(span => span.name === 'GET /burst'); @@ -308,10 +322,11 @@ it('cacheClient: true - burst DO RPC span shares the worker request trace', asyn .unordered() .start(signal); - await runner.makeRequest('get', '/burst?n=1&id=fanout'); - await runner.completed(); + await started.makeRequest('get', `/burst?n=5&id=fanout`); + await started.completed(); expect(workerTraceId).toBeDefined(); + expect(echoSpanCount).toBe(5); expect(echoTraceIds.size).toBe(1); expect([...echoTraceIds][0]).toBe(workerTraceId); }); diff --git a/packages/cloudflare/src/baseSdk.ts b/packages/cloudflare/src/baseSdk.ts index dfbf1ace038b..6275fca2987d 100644 --- a/packages/cloudflare/src/baseSdk.ts +++ b/packages/cloudflare/src/baseSdk.ts @@ -1,5 +1,5 @@ import type { Integration } from '@sentry/core'; -import { getCurrentScope, setCurrentClient } from '@sentry/core'; +import { debug, getCurrentScope, setCurrentClient } from '@sentry/core'; import { consoleIntegration, conversationIdIntegration, @@ -17,6 +17,7 @@ import type { CloudflareClientOptions, CloudflareOptions } from './client'; import { CloudflareClient } from './client'; import { makeFlushLock } from './flush'; import { cacheClient, getCachedClient } from './clientCache'; +import { DEBUG_BUILD } from './debug-build'; import { fetchIntegration } from './integrations/fetch'; import { httpServerIntegration } from './integrations/httpServer'; import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration } from './integrations/spotlight'; @@ -92,16 +93,18 @@ export function initWithDefaultIntegrations( options: CloudflareOptions, getDefaultIntegrationsImpl: (options: CloudflareOptions) => Integration[], ): CloudflareClient | undefined { + const cached = getCachedClient(); const cacheEnabled = options.cacheClient !== false; - if (cacheEnabled) { - const cached = getCachedClient(); - if (cached?.getTransport()) { - getCurrentScope().update(options.initialScope); - setCurrentClient(cached); - cached.setExecutionContext(options.ctx); - return cached; + if (cacheEnabled && cached) { + if (DEBUG_BUILD && cached.getOptions().dsn !== options.dsn) { + debug.warn( + '[Sentry] init() was called with a different DSN than the cached client of this isolate; the cached client keeps its DSN. Pass `cacheClient: false` for per-invocation options.', + ); } + getCurrentScope().update(options.initialScope); + setCurrentClient(cached); + return cached; } if (options.defaultIntegrations === undefined) { @@ -125,7 +128,6 @@ export function initWithDefaultIntegrations( // provider. Scope isolation is handled by the entrypoint wrappers' AsyncLocalStorage strategy. enableOpenTelemetrySetup: options.enableOpenTelemetrySetup ?? false, flushLock, - invocationContext, }; /*! rollup-include-development-only */ diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 68210d6c7bf2..ba072062bf62 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -1,4 +1,4 @@ -import type { Client, ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core'; +import type { ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core'; import { _INTERNAL_clearAiProviderSkips, _INTERNAL_flushLogsBuffer, @@ -11,9 +11,8 @@ import { import { DEBUG_BUILD } from './debug-build'; import type { ExecutionContextCompat } from './executionContext'; import type { makeFlushLock } from './flush'; -import { getOriginalWaitUntil } from './flush'; import type { CloudflareTransportOptions } from './transport'; -import { getInvocationState } from './utils/invocationContext'; +import { getInvocationState, getInvocationWaitUntil } from './utils/invocationContext'; /** * The Sentry Cloudflare SDK Client. @@ -30,7 +29,9 @@ export class CloudflareClient extends ServerRuntimeClient { private _unsubscribeSpanStart: (() => void) | null = null; private _unsubscribeSpanEnd: (() => void) | null = null; - private _invocationContext: ExecutionContextCompat | undefined; + // True while a boundary `flush()` is draining; envelopes created by that drain ride it + // and must not each start their own eager transport flush. + private _inBoundaryFlush = false; /** * Whether this client is a cached, cross-invocation client (`cacheClient`). @@ -39,17 +40,6 @@ export class CloudflareClient extends ServerRuntimeClient { */ public readonly isCachedClient: boolean; - /** - * Points the client at the execution context of the invocation currently being - * served. Called on every invocation for cached clients, since they outlive any - * single invocation. Only a fallback: under concurrency the correct context is - * resolved from the invocation's async context instead (see - * `getInvocationState`), which this field cannot disambiguate. - */ - public setExecutionContext(ctx: ExecutionContextCompat | undefined): void { - this._invocationContext = ctx; - } - /** * Creates a new Cloudflare SDK instance. * @param options Configuration options for this SDK. @@ -57,7 +47,7 @@ export class CloudflareClient extends ServerRuntimeClient { public constructor(options: CloudflareClientOptions) { applySdkMetadata(options, 'cloudflare'); options._metadata = options._metadata || {}; - const { flushLock, invocationContext, ...serverOptions } = options; + const { flushLock, ...serverOptions } = options; const clientOptions: ServerRuntimeClientOptions = { ...serverOptions, @@ -70,15 +60,8 @@ export class CloudflareClient extends ServerRuntimeClient { super(clientOptions); this._flushLock = flushLock; - this._invocationContext = invocationContext; this.isCachedClient = options.cacheClient === true; - if (this.isCachedClient) { - this._setupEagerEnvelopeDelivery(); - this._setupEagerSpanDelivery(); - this._setupEagerLogAndMetricDelivery(); - } - // Track span lifecycle to know when to flush. Skipped for cached clients // (`cacheClient`): they are never disposed, so spans that end after // a flush are still delivered. Per-invocation clients are disposed right after @@ -131,14 +114,6 @@ export class CloudflareClient extends ServerRuntimeClient { * @return {Promise} A promise that resolves to a boolean indicating whether the flush operation was successful. */ public async flush(timeout?: number): Promise { - // Mark this invocation as past its natural flush point: anything captured from - // now on (post-response waitUntil work, detached continuations) has no later - // flush to ride, so it is delivered eagerly (see _setupEagerSpanDelivery). - const invocationState = getInvocationState(); - if (invocationState) { - invocationState.flushPointReached = true; - } - // Wait for user waitUntil-registered work to settle before draining, so events // captured in that work are still in the buffer. Without this the final flush // can drain (and the client be disposed) before background captures land. @@ -146,21 +121,6 @@ export class CloudflareClient extends ServerRuntimeClient { await this._flushLock.finalize(); } - // The eager log/metric drain is debounced to a microtask, so captured logs and - // metrics may not be envelopes yet. Draining only the transport would resolve - // while they are still buffer entries — and a resolving boundary flush lets the - // invocation end before their envelopes are ever created. - if (this.isCachedClient) { - _INTERNAL_flushLogsBuffer(this); - _INTERNAL_flushMetricsBuffer(this); - } - - // Await only drains owned by this invocation. Concurrent invocations keep - // independent chains on their own isolation scopes. - if (invocationState?.eagerFlushPromise) { - await invocationState.eagerFlushPromise; - } - if (this._pendingSpans.size > 0 && this._spanCompletionPromise) { DEBUG_BUILD && debug.log('[CloudflareClient] Waiting for', this._pendingSpans.size, 'pending spans to complete...'); @@ -180,7 +140,15 @@ export class CloudflareClient extends ServerRuntimeClient { await spanCompletionRace; } - return super.flush(timeout); + // Envelopes created while this flush drains (log/metric/span buffers turning into + // envelopes on the `flush` emit) ride its own transport drain; without the flag each + // of them would also start an eager drain and a `waitUntil` registration. + this._inBoundaryFlush = true; + try { + return await super.flush(timeout); + } finally { + this._inBoundaryFlush = false; + } } /** @@ -218,6 +186,25 @@ export class CloudflareClient extends ServerRuntimeClient { // life. Mirrors the same reset in the Node client. _INTERNAL_clearAiProviderSkips(); super._setupIntegrations(); + + // Registered after the integrations on purpose: the flush-point marker must run + // after core and the span buffer have turned their buffers into envelopes on the + // same `flush` emit (so those envelopes ride the boundary drain instead of starting + // eager ones), and the eager span handler relies on `spanStreamingIntegration` + // having already buffered the span that triggers it. + if (this.isCachedClient) { + // Marks the invocation's natural flush point: anything captured from now on + // (post-response `waitUntil` work, detached continuations) has no later flush + // to ride and is delivered eagerly by the hooks below. + this.on('flush', () => { + const invocationState = getInvocationState(); + if (invocationState) { + invocationState.flushPointReached = true; + } + }); + this._setupEagerEnvelopeDelivery(); + this._setupEagerBufferDelivery(); + } } /** @@ -230,157 +217,79 @@ export class CloudflareClient extends ServerRuntimeClient { } /** - * Drains the transport after an envelope has been accepted. - * - * The Cloudflare transport queues request producers until `flush()` is called. Cached - * clients cannot rely on a later invocation boundary, so each accepted envelope starts - * an eager drain. Drains are serialized per invocation and registered with that - * invocation's `waitUntil`, ensuring the runtime keeps their fetches alive after the - * response is returned. + * Drains the transport after an envelope has been accepted past the invocation's + * flush point. The transport queues request producers until `flush()`; before the + * flush point the boundary `flush()` drains everything, but envelopes created after + * it (post-response `waitUntil` work, detached continuations) have no later flush to + * ride. The drain is registered with the invocation's `waitUntil` to keep the fetch + * alive; outside any instrumented invocation it always runs. */ private _setupEagerEnvelopeDelivery(): void { this.on('afterEnvelope', () => { const transport = this.getTransport(); - if (!transport) { - return; - } const invocationState = getInvocationState(); - const flushTransport = (): PromiseLike => transport.flush(2000); - const flushPromise = invocationState?.eagerFlushPromise - ? Promise.resolve(invocationState.eagerFlushPromise).then(flushTransport, flushTransport) - : flushTransport(); - - if (invocationState) { - invocationState.eagerFlushPromise = flushPromise; - void Promise.resolve(flushPromise).finally(() => { - if (invocationState.eagerFlushPromise === flushPromise) { - invocationState.eagerFlushPromise = undefined; - } - }); + + if (!transport || this._inBoundaryFlush || (invocationState && !invocationState.flushPointReached)) { + return; } - this._registerWithInvocationWaitUntil(flushPromise); + this._registerWithInvocationWaitUntil(transport.flush(2000)); }); } /** - * Delivers spans that end after the invocation's flush point. - * - * Spans ending while the invocation is in flight batch in the span buffer and - * are drained by the boundary `flush()` — nothing to do here. Spans ending - * after it (in `waitUntil` work or detached continuations) have no later - * natural flush point, and the buffer's own 5s flush timer would fire outside - * any invocation, where the send can only be registered with a stale execution - * context (or none), and the runtime suspends it. Those traces are flushed - * directly — only their own bucket, never the whole buffer, so a fan-out of - * concurrent traces stays one envelope per trace. - * - * The flush point is per invocation. In Durable Objects it lands at RPC-method - * settle, so RPC spans (which end before it) keep batching one envelope per - * trace — flushing them per call would turn a fan-out trace into one envelope - * per RPC — while detached continuations inheriting that invocation's state - * flush eagerly. + * Turns spans, logs and metrics captured past the invocation's flush point into + * envelopes. Before the flush point the boundary `flush()` drains their buffers; after + * it (or outside any instrumented invocation) nothing else would: the log/metric idle + * timer is disabled for this runtime (`_flushInterval: 0`) and the span buffer's own 5s + * timer fires outside any invocation, where the runtime suspends the send. Each capture + * drains only its own bucket (the ended span's trace, the log buffer, the metric + * buffer), so a fan-out of concurrent traces stays one envelope per trace. RPC method + * spans never reach this path: `wrapMethodWithSentry` runs the boundary flush after the + * method span has ended, so only work past that (e.g. detached continuations) lands here. */ - private _setupEagerSpanDelivery(): void { + private _setupEagerBufferDelivery(): void { this.on('afterSpanEnd', span => { - const invocationState = getInvocationState(); - // Only deliver spans that end after the invocation's flush point — spans - // ending before it batch in the buffer and are drained by the boundary - // flush. RPC sub-invocations in Durable Objects never reach a flush point, - // so their spans batch one envelope per trace here. - if (!invocationState?.flushPointReached) { - return; - } - const pendingTraceIds = (invocationState.pendingSpanFlushTraceIds ??= new Set()); - if (pendingTraceIds.has(span.spanContext().traceId)) { - return; - } - pendingTraceIds.add(span.spanContext().traceId); // The trace id must come from the span, not the current scope: `continueTrace` // writes the propagation context to the *current* scope, so the forked // isolation scope's propagation context carries a different trace id and // flushing by it silently no-ops. const traceId = span.spanContext().traceId; - // Defer to a microtask: a synchronous flush here runs before the span - // streaming integration's own `afterSpanEnd` handler has added the - // triggering span to the buffer (it is registered after this one), so the - // tail span of the invocation would be left behind. The microtask still - // runs in the same async context, so the send stays attributed to this - // invocation. - queueMicrotask(() => { - pendingTraceIds.delete(traceId); - this.emit('flushTraceSpans', traceId); - }); + this._eagerDrain(() => this.emit('flushTraceSpans', traceId)); }); + this.on('afterCaptureLog', () => this._eagerDrain(() => _INTERNAL_flushLogsBuffer(this))); + this.on('afterCaptureMetric', () => this._eagerDrain(() => _INTERNAL_flushMetricsBuffer(this))); } /** - * Turns log and metric captures into envelopes without waiting for a flush. - * - * Unlike events, logs and metrics batch client-side and only become an envelope when - * their buffer is drained. The idle drain timer is disabled for this runtime - * (`_flushInterval: 0`), and a cached client never reaches an invocation-boundary - * `flush()`, so without this a captured log or metric is never delivered at all. - * - * The buffers are drained directly rather than via `emit('flush')`, which would also - * flush an opt-in span buffer mid-invocation and fragment span segments. Draining is - * debounced to a microtask so a synchronous burst (e.g. a loop of `logger` calls) - * still produces a single envelope. + * Runs `drain` unless the owning invocation has not reached its flush point yet + * (before it the boundary `flush()` delivers the buffers). */ - private _setupEagerLogAndMetricDelivery(): void { - let scheduled = false; - const scheduleDrain = (): void => { - if (scheduled) { - return; - } - scheduled = true; - queueMicrotask(() => { - scheduled = false; - _INTERNAL_flushLogsBuffer(this); - _INTERNAL_flushMetricsBuffer(this); - }); - }; - - this.on('afterCaptureLog', scheduleDrain); - this.on('afterCaptureMetric', scheduleDrain); - } + private _eagerDrain(drain: () => void): void { + const invocationState = getInvocationState(); - /** - * Registers every envelope send as tracked I/O with the capturing invocation's - * `waitUntil`. - * - * The SDK never awaits `sendEnvelope()` promises, so an envelope's fetch can be - * pending-but-untracked when the invocation's tracked work settles the runtime - * suspends it and the envelope is lost even though the send started while the - * invocation was still open. This is the dominant loss path for the last captures - * of an invocation (the root span, post-response `waitUntil` work). - */ - public override sendEnvelope(envelope: Parameters[0]): ReturnType { - const sendPromise = super.sendEnvelope(envelope); - if (this.isCachedClient) { - this._registerWithInvocationWaitUntil(sendPromise); + if (invocationState && !invocationState.flushPointReached) { + return; } - return sendPromise; + + drain(); } /** * Attaches a promise to the `waitUntil` of the invocation that owns the current - * async context. The invocation state identifies that invocation even under - * concurrency the fallback field would point at whichever invocation last - * called `init()`, which is the wrong one when invocations overlap. In Durable - * Objects `waitUntil` is a no-op, so this degrades to the same fire-and-forget - * behavior as before there. + * async context, so the runtime keeps the send alive after the response is returned. + * Outside any instrumented invocation (or where the context has no `waitUntil`, + * e.g. Astro prerendering) the send is fire-and-forget. */ private _registerWithInvocationWaitUntil(promise: PromiseLike): void { - const ctx = getInvocationState()?.ctx ?? this._invocationContext; - - if (!ctx) { + const invocationState = getInvocationState(); + const waitUntil = invocationState && getInvocationWaitUntil(invocationState); + if (!waitUntil) { return; } try { - getOriginalWaitUntil(ctx)?.call( - ctx, + waitUntil( Promise.resolve(promise).then( () => undefined, () => undefined, @@ -533,9 +442,14 @@ interface BaseCloudflareOptions { * handlers in that isolate. This avoids the per-invocation cost of * constructing a new client. * - * Since a cached client outlives any single invocation, delivery cannot rely - * on end-of-invocation flushes: captured events are flushed eagerly as they are - * captured, so data captured in detached/background work is still delivered. + * Since a cached client outlives any single invocation, data captured after an + * invocation's flush point (post-response `waitUntil` work, detached background + * work) is delivered eagerly instead of waiting for a flush that never comes. + * + * The first `init()` in an isolate decides the cached client's options; later + * `init()` calls with different options (e.g. another DSN) reuse the cached client + * unchanged (a debug warning is logged for a DSN change). Use `cacheClient: false` + * when options must differ per invocation. * * When disabled, a new client is created per invocation and disposed after the * handler completes. @@ -561,5 +475,4 @@ export interface CloudflareOptions extends Options, */ export interface CloudflareClientOptions extends ClientOptions, BaseCloudflareOptions { flushLock?: ReturnType; - invocationContext?: ExecutionContextCompat; } diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index 0ef61022b060..80a6c6aaf2b5 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -17,6 +17,7 @@ import type { CloudflareClient, CloudflareOptions } from './client'; import type { ExecutionContextCompat } from './executionContext'; import { flushAndDispose, getOriginalWaitUntil } from './flush'; import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils'; +import { getInvocationState, getInvocationWaitUntil } from './utils/invocationContext'; import { withInvocationIsolationScope } from './utils/invocationScope'; import { classifyResponseStreaming } from './utils/streaming'; @@ -76,12 +77,15 @@ export function wrapRequestHandlerWithInit( const { options, request, captureErrors = true } = wrapperOptions; const context = wrapperOptions.context; - // Use getOriginalWaitUntil to get the un-instrumented waitUntil function. - // This is crucial to avoid deadlock: the flush lock mechanism wraps waitUntil - // to track pending tasks. If we use the instrumented version for flushAndDispose, - // it acquires the lock, then flushAndDispose tries to wait for the same lock, - // creating a deadlock. - const waitUntil = context ? getOriginalWaitUntil(context).bind(context) : undefined; + // The un-instrumented waitUntil, so flushAndDispose cannot deadlock on the flush + // lock's own wrapper. Resolved once per invocation and cached on its state; reading + // `ctx.waitUntil` on a native context is a runtime getter call. + const invocationState = getInvocationState(); + const waitUntil = invocationState + ? getInvocationWaitUntil(invocationState) + : context + ? getOriginalWaitUntil(context).bind(context) + : undefined; const errorMechanismType = getRequestErrorMechanismType(context); const client = initSdk({ ...options, ctx: context }); diff --git a/packages/cloudflare/src/utils/invocationContext.ts b/packages/cloudflare/src/utils/invocationContext.ts index 87cdce70c089..5c343c3bd88a 100644 --- a/packages/cloudflare/src/utils/invocationContext.ts +++ b/packages/cloudflare/src/utils/invocationContext.ts @@ -1,6 +1,9 @@ +import type { ExecutionContext } from '@cloudflare/workers-types'; import type { Scope } from '@sentry/core'; -import { getDefaultIsolationScope, getIsolationScope } from '@sentry/core'; +import { debug, getDefaultIsolationScope, getIsolationScope } from '@sentry/core'; +import { DEBUG_BUILD } from '../debug-build'; import type { ExecutionContextCompat } from '../executionContext'; +import { getOriginalWaitUntil } from '../flush'; /** * State owned by a single invocation (request, RPC call, cron, ...). @@ -20,30 +23,46 @@ export interface InvocationState { /** * The execution context of the invocation that owns this scope. Eager envelope * sends are registered with this context's `waitUntil`, so they are attributed to - * the invocation that captured the data — even when a concurrent invocation has - * since pointed the shared client at its own context. + * the invocation that captured the data even when invocations overlap. */ readonly ctx: ExecutionContextCompat | undefined; /** - * Set by `CloudflareClient.flush()` — the invocation's natural flush point. - * Spans ending before it are drained by that flush; spans ending after it (in + * Set on the client's `flush` hook, the invocation's natural flush point. + * Captures before it are drained by that flush; captures after it (in * `waitUntil` work or detached continuations) have no later flush to ride and * are delivered eagerly. */ flushPointReached?: boolean; /** - * Set while an eager span flush is scheduled for this invocation. Kept per - * invocation (not on the shared client) so two concurrent invocations past - * their flush point each schedule their own flush in their own async context — - * the flush and its envelope send stay attributed to the owning invocation. + * `ctx.waitUntil` resolved once for this invocation (see `getInvocationWaitUntil`). + * `null` once resolved to "no usable waitUntil". */ - pendingSpanFlushTraceIds?: Set; - /** Eager transport drains owned by this invocation, serialized in capture order. */ - eagerFlushPromise?: PromiseLike; + waitUntil?: ExecutionContext['waitUntil'] | null; } const INVOCATION_STATE: unique symbol = Symbol('sentryInvocationState'); +/** + * Returns the invocation's original (un-instrumented) `waitUntil`, bound to its context, + * resolving it on first use and caching it on the state. Reading `ctx.waitUntil` on a + * native `ExecutionContext`/`DurableObjectState` goes through the runtime's property + * getter every time; one read per invocation is enough. + */ +export function getInvocationWaitUntil(state: InvocationState): ExecutionContext['waitUntil'] | undefined { + if (state.waitUntil === undefined) { + const ctx = state.ctx; + let resolved: ExecutionContext['waitUntil'] | null = null; + try { + const original = ctx && getOriginalWaitUntil(ctx); + resolved = original ? original.bind(ctx) : null; + } catch { + // Accessing `waitUntil` can throw on foreign or already-torn-down contexts. + } + state.waitUntil = resolved; + } + return state.waitUntil ?? undefined; +} + type ScopeWithInvocationState = Scope & { [INVOCATION_STATE]?: InvocationState; }; @@ -55,6 +74,10 @@ type ScopeWithInvocationState = Scope & { */ export function setInvocationState(scope: Scope, state: InvocationState): void { if (scope === getDefaultIsolationScope()) { + DEBUG_BUILD && + debug.warn( + '[Sentry] Cannot track this invocation. Telemetry captured after the invocation ends may not be delivered.', + ); return; } diff --git a/packages/cloudflare/src/workflows.ts b/packages/cloudflare/src/workflows.ts index 09a639c84c91..e0aa04dd3b8b 100644 --- a/packages/cloudflare/src/workflows.ts +++ b/packages/cloudflare/src/workflows.ts @@ -1,6 +1,6 @@ import { CODE_FUNCTION_NAME, SENTRY_OP } from '@sentry/conventions/attributes'; import { GENERAL_FUNCTION_SPAN_OP } from '@sentry/conventions/op'; -import type { PropagationContext } from '@sentry/core'; +import type { PropagationContext, Scope } from '@sentry/core'; import { captureException, flush, @@ -31,6 +31,7 @@ import { addCloudResourceContext } from './scope-utils'; import { init } from './sdk'; import { instrumentContext } from './utils/instrumentContext'; import type { DefaultEnv, ResolveEnv, StrictCloudflareOptions } from './types'; +import { withInvocationIsolationScope } from './utils/invocationScope'; const UUID_REGEX = /^[0-9a-f]{8}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{12}$/i; @@ -72,6 +73,7 @@ class WrappedWorkflowStep implements WorkflowStep { private _options: CloudflareOptions, private _step: WorkflowStep, private _waitUntil: ExecutionContext['waitUntil'], + private _isolationScope: Scope, ) {} public async do>( @@ -118,40 +120,55 @@ class WrappedWorkflowStep implements WorkflowStep { // or when step context is unavailable (legacy behavior - capture all errors) const isFinalAttempt = !hasStepContext || attempt > retryLimit; - return startSpan( - { - name, - scope: scopeForStep, - attributes: { - [SENTRY_OP]: GENERAL_FUNCTION_SPAN_OP, - [CODE_FUNCTION_NAME]: name, - 'workflow.step.name': name, - 'cloudflare.workflow.timeout': config?.timeout, - 'cloudflare.workflow.retries.backoff': config?.retries?.backoff, - // In workers-types v5, `delay` may be a `WorkflowDelayFunction`, which isn't a valid span attribute value. - 'cloudflare.workflow.retries.delay': - typeof config?.retries?.delay === 'function' ? undefined : config?.retries?.delay, - 'cloudflare.workflow.retries.limit': config?.retries?.limit, - 'cloudflare.workflow.attempt': attempt, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.faas.cloudflare.workflow', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', + // The engine invokes step callbacks outside the async context of `run`, so the + // run's isolation scope (and with it the invocation state that ties eager sends + // to this invocation's `waitUntil`) has to be restored explicitly. + return withIsolationScope(this._isolationScope, () => { + const stepResult = startSpan( + { + name, + scope: scopeForStep, + attributes: { + [SENTRY_OP]: GENERAL_FUNCTION_SPAN_OP, + [CODE_FUNCTION_NAME]: name, + 'workflow.step.name': name, + 'cloudflare.workflow.timeout': config?.timeout, + 'cloudflare.workflow.retries.backoff': config?.retries?.backoff, + // In workers-types v5, `delay` may be a `WorkflowDelayFunction`, which isn't a valid span attribute value. + 'cloudflare.workflow.retries.delay': + typeof config?.retries?.delay === 'function' ? undefined : config?.retries?.delay, + 'cloudflare.workflow.retries.limit': config?.retries?.limit, + 'cloudflare.workflow.attempt': attempt, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.faas.cloudflare.workflow', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', + }, }, - }, - async span => { - try { - const result = await (userCallback as (...args: unknown[]) => Promise)(...args); - span.setStatus({ code: 1 }); - return result; - } catch (error) { - if (isFinalAttempt) { - captureException(error, { mechanism: { handled: true, type: 'auto.faas.cloudflare.workflow' } }); + async span => { + try { + const result = await (userCallback as (...args: unknown[]) => Promise)(...args); + span.setStatus({ code: 1 }); + return result; + } catch (error) { + if (isFinalAttempt) { + captureException(error, { mechanism: { handled: true, type: 'auto.faas.cloudflare.workflow' } }); + } + throw error; } - throw error; - } finally { + }, + ); + // Deliver after the step span has ended, so the span rides this flush instead of + // starting an eager drain (same ordering as `wrapMethodWithSentry`'s teardown). + return stepResult.then( + result => { this._waitUntil(flush(2000)); - } - }, - ); + return result; + }, + error => { + this._waitUntil(flush(2000)); + throw error; + }, + ); + }); }; if (config) { @@ -230,7 +247,7 @@ export function instrumentWorkflowWithSentry< return async function (event: WorkflowEvent

, step: WorkflowStep): Promise { setAsyncLocalStorageAsyncContextStrategy(); - return withIsolationScope(async isolationScope => { + return withInvocationIsolationScope(async isolationScope => { const waitUntil = getOriginalWaitUntil(context).bind(context); const client = init({ ...options, ctx: context, enableDedupe: false }); isolationScope.setClient(client); @@ -245,13 +262,13 @@ export function instrumentWorkflowWithSentry< return await obj.run.call( obj, event, - new WrappedWorkflowStep(event.instanceId, options, step, waitUntil), + new WrappedWorkflowStep(event.instanceId, options, step, waitUntil, isolationScope), ); } finally { waitUntil(flushAndDispose(client)); } }); - }); + }, context); }; } return Reflect.get(obj, prop, receiver); diff --git a/packages/cloudflare/src/wrapMethodWithSentry.ts b/packages/cloudflare/src/wrapMethodWithSentry.ts index e15d4cd7d5b6..8e6f2f44c951 100644 --- a/packages/cloudflare/src/wrapMethodWithSentry.ts +++ b/packages/cloudflare/src/wrapMethodWithSentry.ts @@ -16,6 +16,7 @@ import type { ExecutionContextCompat } from './executionContext'; import { flushAndDispose, getOriginalWaitUntil } from './flush'; import { ensureInstrumented } from './instrument'; import { init } from './sdk'; +import { getInvocationState, getInvocationWaitUntil } from './utils/invocationContext'; import { withInvocationIsolationScope } from './utils/invocationScope'; import { extractRpcMeta } from './utils/rpcMeta'; import { buildSpanLinks, getStoredSpanContext, storeSpanContext } from './utils/traceLinks'; @@ -117,16 +118,22 @@ export function wrapMethodWithSentry( const context: typeof wrapperOptions.context | undefined = wrapperOptions.context; // see: https://github.com/getsentry/sentry-javascript/issues/22328 - const waitUntil = context - ? getOriginalWaitUntil(context as ExecutionContextCompat)?.bind(context) - : undefined; + // Resolved once per invocation and cached on its state: reading `ctx.waitUntil` + // on a native context is a runtime getter call. + const invocationState = getInvocationState(); + const waitUntil = invocationState + ? getInvocationWaitUntil(invocationState) + : context + ? getOriginalWaitUntil(context as ExecutionContextCompat)?.bind(context) + : undefined; const storage = resolveOriginalStorage(context, thisArg); let scopeClient = scope.getClient(); - // Check if client exists AND is still usable (transport not disposed) - // This handles the case where a previous handler disposed the client - // but the scope still holds a reference to it (e.g., alarm handlers in Durable Objects) - // For startNewTrace, always create a fresh client + // Re-init when the scope has no usable client (a previous handler disposed it, e.g. + // DO alarm handlers) or for `startNewTrace`, so the client points at this + // invocation's context. With `cacheClient: true` `init()` returns the isolate's + // cached client and only creates (and caches) a fresh one when nothing is cached yet; + // `cacheClient: false` creates one per invocation. if (startNewTrace || !scopeClient?.getTransport()) { const client = init({ ...wrapperOptions.options, @@ -151,17 +158,24 @@ export function wrapMethodWithSentry( return res; }; - const onRejected = (e: unknown) => { + const captureAndRethrow = (e: unknown): never => { captureException(e, { mechanism: { type: origin, handled: false, }, }); - waitUntil?.(teardown()); throw e; }; + const onRejected = (e: unknown) => { + try { + return captureAndRethrow(e); + } finally { + waitUntil?.(teardown()); + } + }; + if (!wrapperOptions.spanName) { try { if (callback) { @@ -210,28 +224,58 @@ export function wrapMethodWithSentry( const result = Reflect.apply(target, thisArg, args); if (isThenable(result)) { - return result.then(onFulfilled, onRejected); - } else { - return onFulfilled(result); + return result.then(undefined, captureAndRethrow); } + return result; } catch (e) { - return onRejected(e); + return captureAndRethrow(e); } }); }; + // The boundary flush (teardown) must run after the method span has ended, so the + // span is in the buffer when that flush drains it. Running it inside the span + // callback (before `startSpan` ends the span) would leave the span for the eager + // path and cost a second flush per invocation. + const runWithTeardown = (run: () => unknown): unknown => { + let out: unknown; + try { + out = run(); + } catch (e) { + // Synchronous throw: `startSpan` already ended the span and rethrew. + waitUntil?.(teardown()); + throw e; + } + if (isThenable(out)) { + return out.then( + res => { + waitUntil?.(teardown()); + return res; + }, + e => { + waitUntil?.(teardown()); + throw e; + }, + ); + } + waitUntil?.(teardown()); + return out; + }; + if (rpcMeta) { - return continueTrace( - { sentryTrace: rpcMeta['sentry-trace'] || '', baggage: rpcMeta.baggage || '' }, - executeSpan, + return runWithTeardown(() => + continueTrace( + { sentryTrace: rpcMeta['sentry-trace'] || '', baggage: rpcMeta.baggage || '' }, + executeSpan, + ), ); } if (startNewTrace) { - return startNewTraceCore(() => executeSpan()); + return runWithTeardown(() => startNewTraceCore(() => executeSpan())); } - return executeSpan(); + return runWithTeardown(executeSpan); }; return withInvocationIsolationScope( diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 9687ec6140e3..09bff574e479 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -409,276 +409,162 @@ describe('CloudflareClient', () => { }); }); - describe('cached client eager flush tracking', () => { - function makeEagerFlushClient(flushMock: ReturnType): CloudflareClient { - return new CloudflareClient({ + describe('cached client eager envelope delivery', () => { + function makeEagerFlushClient( + flushMock: ReturnType, + extra: Partial = {}, + ): CloudflareClient { + const client = new CloudflareClient({ ...MOCK_CLIENT_OPTIONS, cacheClient: true, transport: () => ({ send: vi.fn().mockResolvedValue({}), flush: flushMock, }), + ...extra, }); + client.init(); + return client; } - it('flush() awaits in-flight eager envelope flushes', async () => { - let resolveEagerFlush: (value: boolean) => void = () => undefined; - let call = 0; - const flushMock = vi.fn().mockImplementation(() => { - call++; - if (call === 1) { - return new Promise(res => { - resolveEagerFlush = res; - }); - } - return Promise.resolve(true); - }); + function reachFlushPoint(): void { + const state = getInvocationState(); + if (state) { + state.flushPointReached = true; + } + } + + it('does not drain the transport for envelopes before the invocation flush point', () => { + const flushMock = vi.fn().mockResolvedValue(true); const client = makeEagerFlushClient(flushMock); const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; - await withInvocationIsolationScope(async () => { - // An emitted envelope starts this invocation's eager transport drain. + withInvocationIsolationScope(() => { client.emit('afterEnvelope', {}); - expect(flushMock).toHaveBeenCalledTimes(1); - - let flushResolved = false; - const flushPromise = client.flush(10).then(result => { - flushResolved = true; - return result; - }); - - // The boundary flush waits for the drain owned by this invocation. - await new Promise(resolve => setTimeout(resolve, 20)); - expect(flushResolved).toBe(false); - - resolveEagerFlush(true); - await expect(flushPromise).resolves.toBe(true); - expect(flushResolved).toBe(true); }, ctx as never); - }); - - it('flush() resolves immediately once eager flushes have settled', async () => { - const flushMock = vi.fn().mockResolvedValue(true); - const client = makeEagerFlushClient(flushMock); - - client.emit('afterEnvelope', {}); - expect(flushMock).toHaveBeenCalledTimes(1); - await expect(client.flush(10)).resolves.toBe(true); + expect(flushMock).not.toHaveBeenCalled(); + expect(ctx.waitUntil).not.toHaveBeenCalled(); }); - it('tracks eager drains independently for concurrent invocations', async () => { - let resolveA: (value: boolean) => void = () => undefined; - let resolveB: (value: boolean) => void = () => undefined; - let call = 0; - const flushMock = vi.fn().mockImplementation(() => { - call++; - if (call === 1) { - return new Promise(resolve => { - resolveA = resolve; - }); - } - if (call === 2) { - return new Promise(resolve => { - resolveB = resolve; - }); - } - return Promise.resolve(true); - }); + it('drains the transport for envelopes past the flush point and registers it with the invocation waitUntil', () => { + const flushMock = vi.fn().mockResolvedValue(true); const client = makeEagerFlushClient(flushMock); - const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; - const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; - let flushAResolved = false; - let flushBResolved = false; - const flushA = withInvocationIsolationScope(async () => { - client.emit('afterEnvelope', {}); - return client.flush(1000).then(result => { - flushAResolved = true; - return result; - }); - }, ctxA as never); - const flushB = withInvocationIsolationScope(async () => { + withInvocationIsolationScope(() => { + reachFlushPoint(); client.emit('afterEnvelope', {}); - return client.flush(1000).then(result => { - flushBResolved = true; - return result; - }); - }, ctxB as never); - - resolveA(true); - await expect(flushA).resolves.toBe(true); - expect(flushAResolved).toBe(true); - expect(flushBResolved).toBe(false); + }, ctx as never); - resolveB(true); - await expect(flushB).resolves.toBe(true); - expect(flushBResolved).toBe(true); + expect(flushMock).toHaveBeenCalledTimes(1); + expect(ctx.waitUntil).toHaveBeenCalledTimes(1); + expect(ctx.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); }); - it('does not flush eagerly per envelope when cacheClient is disabled', () => { + it('drains the transport for envelopes outside any invocation', () => { const flushMock = vi.fn().mockResolvedValue(true); - const client = new CloudflareClient({ - ...MOCK_CLIENT_OPTIONS, - cacheClient: false, - transport: () => ({ - send: vi.fn().mockResolvedValue({}), - flush: flushMock, - }), - }); + const client = makeEagerFlushClient(flushMock); client.emit('afterEnvelope', {}); - expect(flushMock).not.toHaveBeenCalled(); - }); - - it('registers the eager flush with the invocation context waitUntil', () => { - const flushMock = vi.fn().mockResolvedValue(true); - const waitUntil = vi.fn(); - const ctx = { waitUntil, passThroughOnException: vi.fn() }; - const client = new CloudflareClient({ - ...MOCK_CLIENT_OPTIONS, - cacheClient: true, - // oxlint-disable-next-line typescript/no-explicit-any - invocationContext: ctx as any, - transport: () => ({ - send: vi.fn().mockResolvedValue({}), - flush: flushMock, - }), - }); - client.emit('afterEnvelope', {}); expect(flushMock).toHaveBeenCalledTimes(1); - expect(waitUntil).toHaveBeenCalledTimes(1); }); - it('uses the context from setExecutionContext for eager flush registration', () => { + it('does not register a waitUntil when no context is known', () => { const flushMock = vi.fn().mockResolvedValue(true); - const waitUntil = vi.fn(); - const ctx = { waitUntil, passThroughOnException: vi.fn() }; - const client = new CloudflareClient({ - ...MOCK_CLIENT_OPTIONS, - cacheClient: true, - transport: () => ({ - send: vi.fn().mockResolvedValue({}), - flush: flushMock, - }), - }); + const client = makeEagerFlushClient(flushMock); - // oxlint-disable-next-line typescript/no-explicit-any - client.setExecutionContext(ctx as any); - client.emit('afterEnvelope', {}); - expect(waitUntil).toHaveBeenCalledTimes(1); + expect(() => client.emit('afterEnvelope', {})).not.toThrow(); + expect(flushMock).toHaveBeenCalledTimes(1); }); - it('registers the eager flush with the capturing invocation, not the latest one', () => { + it('registers the drain with the capturing invocation, not the latest one', () => { const flushMock = vi.fn().mockResolvedValue(true); - const waitUntilA = vi.fn(); - const waitUntilB = vi.fn(); - const ctxA = { waitUntil: waitUntilA, passThroughOnException: vi.fn() }; - const ctxB = { waitUntil: waitUntilB, passThroughOnException: vi.fn() }; + const ctxA = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + const ctxB = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; const client = makeEagerFlushClient(flushMock); - // The shared client's fallback points at the latest invocation (B). An - // envelope captured by the still-running invocation A must register its - // flush on A's waitUntil — otherwise it is suspended when B's invocation - // ends first. - // oxlint-disable-next-line typescript/no-explicit-any - client.setExecutionContext(ctxB as any); - + // An envelope captured by invocation A must register its drain on A's + // waitUntil, even when invocation B is the one that ran `init()` last. withInvocationIsolationScope(() => { + reachFlushPoint(); client.emit('afterEnvelope', {}); }, ctxA as never); - expect(waitUntilA).toHaveBeenCalledTimes(1); - expect(waitUntilB).not.toHaveBeenCalled(); + expect(ctxA.waitUntil).toHaveBeenCalledTimes(1); + expect(ctxB.waitUntil).not.toHaveBeenCalled(); withInvocationIsolationScope(() => { + reachFlushPoint(); client.emit('afterEnvelope', {}); }, ctxB as never); - expect(waitUntilB).toHaveBeenCalledTimes(1); + expect(ctxB.waitUntil).toHaveBeenCalledTimes(1); }); - it('registers envelope sends with the capturing invocation waitUntil', () => { - const waitUntil = vi.fn(); - const ctx = { waitUntil, passThroughOnException: vi.fn() }; - const sendMock = vi.fn().mockResolvedValue({}); - const client = new CloudflareClient({ - ...MOCK_CLIENT_OPTIONS, - cacheClient: true, - transport: () => ({ - send: sendMock, - flush: vi.fn().mockResolvedValue(true), - }), + it('never lets a failing drain reject the waitUntil registration', async () => { + let registered: Promise | undefined; + const waitUntil = vi.fn((promise: Promise) => { + registered = promise; }); - - withInvocationIsolationScope(() => { - void client.sendEnvelope([{}, []] as never); - }, ctx as never); - - expect(sendMock).toHaveBeenCalledTimes(1); - expect(waitUntil).toHaveBeenCalledTimes(1); - }); - - it('does not register sends with waitUntil when cacheClient is disabled', () => { - const waitUntil = vi.fn(); const ctx = { waitUntil, passThroughOnException: vi.fn() }; - const sendMock = vi.fn().mockResolvedValue({}); - const client = new CloudflareClient({ - ...MOCK_CLIENT_OPTIONS, - cacheClient: false, - transport: () => ({ - send: sendMock, - flush: vi.fn().mockResolvedValue(true), - }), - }); + const client = makeEagerFlushClient(vi.fn().mockRejectedValue(new Error('ingest down'))); withInvocationIsolationScope(() => { - void client.sendEnvelope([{}, []] as never); + reachFlushPoint(); + client.emit('afterEnvelope', {}); }, ctx as never); - expect(sendMock).toHaveBeenCalledTimes(1); - expect(waitUntil).not.toHaveBeenCalled(); + expect(waitUntil).toHaveBeenCalledTimes(1); + // The promise handed to the runtime must resolve: a rejected waitUntil + // promise would mark the invocation's outcome as an exception. + await expect(registered).resolves.toBeUndefined(); }); - it('never lets a failing send reject the waitUntil registration', async () => { - let registered: Promise | undefined; - const waitUntil = vi.fn((promise: Promise) => { - registered = promise; - }); - const ctx = { waitUntil, passThroughOnException: vi.fn() }; + it('envelopes created by the boundary flush ride its drain; later ones start their own', async () => { + const flushMock = vi.fn().mockResolvedValue(true); const client = new CloudflareClient({ ...MOCK_CLIENT_OPTIONS, cacheClient: true, - transport: () => ({ - send: vi.fn().mockRejectedValue(new Error('ingest down')), - flush: vi.fn().mockResolvedValue(true), - }), + transport: () => ({ send: vi.fn().mockResolvedValue({}), flush: flushMock }), + }); + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + let envelopesDuringFlush = 0; + // Hooks registered before the client's own (core log/metric drains, the span + // buffer, which `init()` sets up) create envelopes on the same `flush` emit. + client.on('flush', () => { + client.emit('afterEnvelope', {}); + envelopesDuringFlush = flushMock.mock.calls.length; }); + client.init(); - withInvocationIsolationScope(() => { - void client.sendEnvelope([{}, []] as never); - }, ctx as never); + await withInvocationIsolationScope(async () => { + await client.flush(10); + // No eager drain for that envelope: the flush point was not marked yet when it + // was created, and the boundary flush's own transport drain follows. + expect(envelopesDuringFlush).toBe(0); + expect(flushMock).toHaveBeenCalledTimes(1); + expect(ctx.waitUntil).not.toHaveBeenCalled(); - expect(waitUntil).toHaveBeenCalledTimes(1); - // The promise handed to the runtime must resolve — a rejected waitUntil - // promise would mark the invocation's outcome as an exception. - await expect(registered).resolves.toBeUndefined(); + client.emit('afterEnvelope', {}); + expect(flushMock).toHaveBeenCalledTimes(2); + expect(ctx.waitUntil).toHaveBeenCalledTimes(1); + }, ctx as never); }); - it('does not register a waitUntil when no invocation context is set', () => { + it('does not flush eagerly per envelope when cacheClient is disabled', () => { const flushMock = vi.fn().mockResolvedValue(true); - const waitUntil = vi.fn(); const client = new CloudflareClient({ ...MOCK_CLIENT_OPTIONS, - cacheClient: true, + cacheClient: false, transport: () => ({ send: vi.fn().mockResolvedValue({}), flush: flushMock, }), }); + client.init(); client.emit('afterEnvelope', {}); - expect(flushMock).toHaveBeenCalledTimes(1); - expect(waitUntil).not.toHaveBeenCalled(); + expect(flushMock).not.toHaveBeenCalled(); }); }); @@ -689,6 +575,7 @@ describe('CloudflareClient', () => { cacheClient: true, traceLifecycle: 'stream', } as never); + client.init(); const flushSpy = vi.fn(); client.on('flushTraceSpans', flushSpy); return { client, flushSpy }; @@ -790,17 +677,18 @@ describe('CloudflareClient', () => { expect(flushSpy).toHaveBeenCalledWith('trace-b'); }); - it('still collapses repeated span ends of the same trace into one flush', async () => { + it('flushes each span end of the same trace on its own', async () => { + // No per-turn coalescing: draining only the ended span's trace bucket is cheap, and + // deferring it bought no measurable CPU in production. const { client, flushSpy } = makeCachedClient(); await withInvocationIsolationScope(async () => { await client.flush(0); client.emit('afterSpanEnd', makeSpan('trace-a') as never); client.emit('afterSpanEnd', makeSpan('trace-a') as never); - await tick(); }, ctx as never); - expect(flushSpy).toHaveBeenCalledTimes(1); + expect(flushSpy).toHaveBeenCalledTimes(2); expect(flushSpy).toHaveBeenCalledWith('trace-a'); }); @@ -854,13 +742,16 @@ describe('CloudflareClient', () => { expect(flushSpy).toHaveBeenCalledWith('trace-1'); }); - it('does not flush spans ending outside any invocation', async () => { + it('flushes spans ending outside any invocation eagerly', async () => { const { client, flushSpy } = makeCachedClient(); + // No boundary flush will ever come for them, and the buffer's own timer + // would fire where the runtime suspends the send. + client.emit('afterSpanEnd', makeSpan('trace-1') as never); client.emit('afterSpanEnd', makeSpan('trace-1') as never); - await tick(); - expect(flushSpy).not.toHaveBeenCalled(); + expect(flushSpy).toHaveBeenCalledTimes(2); + expect(flushSpy).toHaveBeenCalledWith('trace-1'); }); it('does not flush for span ends when cacheClient is disabled', async () => { diff --git a/packages/cloudflare/test/durableobject.test.ts b/packages/cloudflare/test/durableobject.test.ts index 453b5dfbb8a5..353cc0e1d04a 100644 --- a/packages/cloudflare/test/durableobject.test.ts +++ b/packages/cloudflare/test/durableobject.test.ts @@ -60,10 +60,12 @@ describe('instrumentDurableObjectWithSentry', () => { .mockReturnValueOnce({ orgId: 1, enableRpcTracePropagation: true, + cacheClient: false, }) .mockReturnValueOnce({ orgId: 2, enableRpcTracePropagation: true, + cacheClient: false, }); const testClass = class { method() {} @@ -98,7 +100,10 @@ describe('instrumentDurableObjectWithSentry', () => { const mockEnv = {} as any; const initCore = vi.spyOn(SentryCore, 'initAndBind'); vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined); - const options = vi.fn().mockReturnValueOnce({ orgId: 1 }).mockReturnValueOnce({ orgId: 2 }); + const options = vi + .fn() + .mockReturnValueOnce({ orgId: 1, cacheClient: false }) + .mockReturnValueOnce({ orgId: 2, cacheClient: false }); const testClass = class { webSocketMessage() {} diff --git a/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts b/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts index 116c9e9637fe..4dddbc2b23d5 100644 --- a/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts +++ b/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts @@ -519,8 +519,9 @@ describe('instrumentWorkerEntrypoint', () => { webSocketMessage() {} }; + // A per-invocation client makes every instrumented method call visible as its own init. const obj = Reflect.construct( - instrumentWorkerEntrypoint(() => ({}), TestClass as unknown as WorkerEntrypointConstructor), + instrumentWorkerEntrypoint(() => ({ cacheClient: false }), TestClass as unknown as WorkerEntrypointConstructor), [createMockExecutionContext(), {}], ); diff --git a/packages/cloudflare/test/sdk.test.ts b/packages/cloudflare/test/sdk.test.ts index 1146b5eb8bc7..b8f5c93520c6 100644 --- a/packages/cloudflare/test/sdk.test.ts +++ b/packages/cloudflare/test/sdk.test.ts @@ -121,6 +121,20 @@ describe('cacheClient', () => { expect(second).toBe(first); }); + test('warns when a later init passes a different DSN than the cached client', () => { + const warn = vi.spyOn(SentryCore.debug, 'warn').mockImplementation(() => undefined); + + const first = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + expect(warn).not.toHaveBeenCalled(); + + init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + expect(warn).not.toHaveBeenCalled(); + + const second = init({ dsn: 'https://other@dsn.ingest.sentry.io/4242' }); + expect(second).toBe(first); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('different DSN')); + }); + test('re-binds the cached client to the current scope on repeated init', () => { const options = { dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -137,18 +151,25 @@ describe('cacheClient', () => { expect(getClient()).toBe(cached); }); - test('creates a fresh client when the cached one was disposed', () => { - const options = { - dsn: 'https://public@dsn.ingest.sentry.io/1337', - } as const; + test('caches a client without a DSN and reuses it', () => { + // A disabled (DSN-less) client is still created once per isolate, not per invocation. + const first = init({}); + const second = init({}); - const cached = init({ ...options }); - cached?.dispose(); + expect(first).toBeDefined(); + expect(second).toBe(first); + expect(first?.isCachedClient).toBe(true); + }); - const again = init({ ...options }); - expect(again).toBeDefined(); - expect(again).not.toBe(cached); - expect(again?.getTransport()).toBeDefined(); + test('keeps the cached DSN-less client when a later init passes a DSN', () => { + // First init wins for the isolate, so an isolate whose first init had no DSN (a missing env + // var on one route) stays disabled. `cacheClient: false` is the escape hatch. + const first = init({}); + const second = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + + expect(second).toBe(first); + expect(second?.getOptions().dsn).toBeUndefined(); + expect(second?.getTransport()).toBeUndefined(); }); test('flushes eagerly when an envelope is sent on a cached client', async () => { @@ -220,7 +241,7 @@ describe('cacheClient', () => { await vi.waitFor(() => expect(itemTypes(envelopes)).toContain('trace_metric')); }); - test('coalesces a synchronous burst of logs into a single envelope', async () => { + test('delivers each log captured outside an invocation in its own envelope', async () => { const { envelopes } = initWithCapturingTransport(); for (let i = 0; i < 5; i++) { @@ -228,7 +249,7 @@ describe('cacheClient', () => { } await vi.waitFor(() => expect(itemTypes(envelopes)).toContain('log')); - expect(itemTypes(envelopes).filter(type => type === 'log')).toHaveLength(1); + expect(itemTypes(envelopes).filter(type => type === 'log')).toHaveLength(5); }); test('keeps batching logs until flush for a non-cached client', async () => { diff --git a/packages/cloudflare/test/utils/invocationContext.test.ts b/packages/cloudflare/test/utils/invocationContext.test.ts index d560b21e9707..cd05ebf6b8cd 100644 --- a/packages/cloudflare/test/utils/invocationContext.test.ts +++ b/packages/cloudflare/test/utils/invocationContext.test.ts @@ -1,8 +1,8 @@ -import { getDefaultIsolationScope, getIsolationScope, GLOBAL_OBJ, withIsolationScope } from '@sentry/core'; +import { debug, getDefaultIsolationScope, getIsolationScope, GLOBAL_OBJ, withIsolationScope } from '@sentry/core'; import { AsyncLocalStorage } from 'async_hooks'; import { setAsyncLocalStorageAsyncContextStrategy } from '@sentry/server-utils/no-diagnostic-channels'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { getInvocationState, setInvocationState } from '../../src/utils/invocationContext'; +import { getInvocationState, getInvocationWaitUntil, setInvocationState } from '../../src/utils/invocationContext'; import { withInvocationIsolationScope } from '../../src/utils/invocationScope'; describe('invocation state', () => { @@ -21,6 +21,21 @@ describe('invocation state', () => { }); }); + it('never attaches state to the default isolation scope and warns', () => { + // The stack async-context strategy does not fork, so a wrapper can end up handing the + // default isolation scope to setInvocationState. State on it would be shared by every + // invocation in the isolate, so it must be dropped and read back as "no invocation". + const warnSpy = vi.spyOn(debug, 'warn').mockImplementation(() => undefined); + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + setInvocationState(getDefaultIsolationScope(), { ctx }); + + expect(getInvocationState()).toBeUndefined(); + withIsolationScope(getDefaultIsolationScope(), () => { + expect(getInvocationState()).toBeUndefined(); + }); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Cannot track this invocation')); + }); + it('exposes state attached to the active isolation scope', () => { const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; const scope = getDefaultIsolationScope().clone(); @@ -73,4 +88,46 @@ describe('invocation state', () => { expect(getIsolationScope()).toBe(getDefaultIsolationScope()); expect(getInvocationState()).toBeUndefined(); }); + + it('resolves the context waitUntil once and caches it bound on the state', () => { + const waitUntil = vi.fn(); + const readWaitUntil = vi.fn(() => waitUntil); + const ctx = { + passThroughOnException: vi.fn(), + get waitUntil() { + return readWaitUntil(); + }, + }; + const state = { ctx }; + + const first = getInvocationWaitUntil(state); + const second = getInvocationWaitUntil(state); + + expect(second).toBe(first); + expect(readWaitUntil).toHaveBeenCalledTimes(1); + const promise = Promise.resolve(); + first?.(promise); + expect(waitUntil).toHaveBeenCalledWith(promise); + }); + + it('caches "no usable waitUntil" when reading it throws', () => { + const readWaitUntil = vi.fn(() => { + throw new Error('torn down'); + }); + const ctx = { + passThroughOnException: vi.fn(), + get waitUntil(): never { + return readWaitUntil() as never; + }, + }; + const state = { ctx }; + + expect(getInvocationWaitUntil(state)).toBeUndefined(); + expect(getInvocationWaitUntil(state)).toBeUndefined(); + expect(readWaitUntil).toHaveBeenCalledTimes(1); + }); + + it('returns undefined for a state without a context', () => { + expect(getInvocationWaitUntil({ ctx: undefined })).toBeUndefined(); + }); }); diff --git a/packages/cloudflare/test/workflow.test.ts b/packages/cloudflare/test/workflow.test.ts index 0e36e15ab5d7..bc4ba866d574 100644 --- a/packages/cloudflare/test/workflow.test.ts +++ b/packages/cloudflare/test/workflow.test.ts @@ -1,7 +1,8 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import { startSpan } from '@sentry/core'; +import { getDefaultIsolationScope, getIsolationScope, startSpan, withIsolationScope } from '@sentry/core'; import type { WorkflowEvent, WorkflowStep, WorkflowStepConfig } from 'cloudflare:workers'; import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { getInvocationState } from '../src/utils/invocationContext'; import { deterministicTraceIdFromInstanceId, instrumentWorkflowWithSentry } from '../src/workflows'; import { resetSdk } from './testUtils'; @@ -135,10 +136,9 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('first step', expect.any(Function)); - // We flush after the step.do and at the end of the run, plus one - // waitUntil registration for the eagerly delivered envelope - // and one for the envelope send itself - expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); + // One flush after the step.do (past its span end, so the span rides it, no eager + // registration) and one at the end of the run + expect(mockContext.waitUntil).toHaveBeenCalledTimes(2); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); expect(mockTransport.send).toHaveBeenCalledTimes(1); expect(mockTransport.send).toHaveBeenCalledWith([ @@ -383,10 +383,9 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('first step', expect.any(Function)); - // We flush after the step.do and at the end of the run, plus one - // waitUntil registration for the eagerly delivered envelope - // and one for the envelope send itself - expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); + // One flush after the step.do (past its span end, so the span rides it, no eager + // registration) and one at the end of the run + expect(mockContext.waitUntil).toHaveBeenCalledTimes(2); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); expect(mockTransport.send).toHaveBeenCalledTimes(1); expect(mockTransport.send).toHaveBeenCalledWith([ @@ -459,10 +458,9 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('sometimes error step', expect.any(Function)); - // One flush for the failed attempt, one for the retry success, one at end of run, - // plus one waitUntil registration per eagerly delivered envelope - // and one per envelope send - expect(mockContext.waitUntil).toHaveBeenCalledTimes(7); + // One flush per attempt (failed and retried, past the span end) and one at end of + // run, plus one eager registration for the envelope of the error captured mid-run + expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); // No error event (not final attempt), only failed transaction + successful retry transaction expect(mockTransport.send).toHaveBeenCalledTimes(2); @@ -732,10 +730,8 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { const event = { payload: {}, timestamp: new Date(), instanceId: INSTANCE_ID }; await workflow.run(event, mockStep); - // Flush after step.do and at end of run, plus one - // waitUntil registration for the eagerly delivered envelope - // and one for the envelope send itself - expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); + // One flush after step.do (past its span end) and one at end of run + expect(mockContext.waitUntil).toHaveBeenCalledTimes(2); expect(mockTransport.send).toHaveBeenCalledTimes(1); const sendArg = mockTransport.send.mock.calls[0]![0]; @@ -752,4 +748,38 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(stepSpan).toBeDefined(); expect(stepSpan.parent_span_id).toBe(rootSpanId); }); + + test('step callbacks run on the run isolation scope even when the engine invokes them outside it', async () => { + // The Workflows engine calls step callbacks from its own async context, not from the one + // `run` is executing in. Emulate that by invoking the callback under the default isolation + // scope: without the restore, the step would see neither the run's scope data nor its + // invocation state (and eager sends would have no `waitUntil` to attach to). + const foreignStep: WorkflowStep = { + ...mockStep, + do: vi.fn().mockImplementation(async (_name: string, callback: (...args: unknown[]) => Promise) => + withIsolationScope(getDefaultIsolationScope(), () => callback(MOCK_STEP_CTX)), + ), + }; + let tagInsideStep: unknown; + let hasInvocationStateInsideStep = false; + + class ScopeWorkflow { + constructor(_ctx: ExecutionContext, _env: unknown) {} + + async run(_event: Readonly>, step: WorkflowStep): Promise { + getIsolationScope().setTag('wf.run', 'marker'); + await step.do('scoped step', async () => { + tagInsideStep = getIsolationScope().getScopeData().tags['wf.run']; + hasInvocationStateInsideStep = getInvocationState() !== undefined; + }); + } + } + + const TestWorkflowInstrumented = instrumentWorkflowWithSentry(getSentryOptions, ScopeWorkflow as any); + const workflow = new TestWorkflowInstrumented(mockContext, {}) as ScopeWorkflow; + await workflow.run({ payload: {}, timestamp: new Date(), instanceId: INSTANCE_ID }, foreignStep); + + expect(tagInsideStep).toBe('marker'); + expect(hasInvocationStateInsideStep).toBe(true); + }); }); From eba2d861fbbd8ef594a0107a2d5b064bb13b5c23 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 24 Aug 2026 18:40:11 +0300 Subject: [PATCH 14/14] fixup! ref: review suggestions --- packages/cloudflare/test/workflow.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/cloudflare/test/workflow.test.ts b/packages/cloudflare/test/workflow.test.ts index bc4ba866d574..8a2e1825f83a 100644 --- a/packages/cloudflare/test/workflow.test.ts +++ b/packages/cloudflare/test/workflow.test.ts @@ -756,9 +756,11 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { // invocation state (and eager sends would have no `waitUntil` to attach to). const foreignStep: WorkflowStep = { ...mockStep, - do: vi.fn().mockImplementation(async (_name: string, callback: (...args: unknown[]) => Promise) => - withIsolationScope(getDefaultIsolationScope(), () => callback(MOCK_STEP_CTX)), - ), + do: vi + .fn() + .mockImplementation(async (_name: string, callback: (...args: unknown[]) => Promise) => + withIsolationScope(getDefaultIsolationScope(), () => callback(MOCK_STEP_CTX)), + ), }; let tagInsideStep: unknown; let hasInvocationStateInsideStep = false;