From 384f556d2386b2b559ac306e1d2dea0c8e09ed7b Mon Sep 17 00:00:00 2001 From: Dominic Rubas <1042243+rubas@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:33:28 +0200 Subject: [PATCH 1/4] fix(cloudflare): Keep the invocation state in forked isolation scopes `withMonitor`, `withIsolationScope`, `trpcMiddleware` and `wrapMcpServerWithSentry` fork the isolation scope. `Scope.clone()` did not copy the invocation state, because it was a symbol property on the scope. Inside the fork, the cached client treated each envelope as sent outside any invocation: it drained the transport at once and could not register the send with `waitUntil`. The runtime cancelled the send when the event ended, so the closing check-in of `withMonitor` was lost when it ended a scheduled, fetch or queue handler. The eager drain also started envelopes that concurrent invocations had queued for their own flush. The state now lives in the scope's `refs`, which `clone()` copies by reference, so a fork shares the state of its invocation. --- .../cloudflare/src/utils/invocationContext.ts | 17 ++++---- .../worker/instrumentScheduled.test.ts | 39 +++++++++++++++++++ .../test/utils/invocationContext.test.ts | 15 ++++--- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/packages/cloudflare/src/utils/invocationContext.ts b/packages/cloudflare/src/utils/invocationContext.ts index 5c343c3bd88a..5b80e76ea5e5 100644 --- a/packages/cloudflare/src/utils/invocationContext.ts +++ b/packages/cloudflare/src/utils/invocationContext.ts @@ -16,8 +16,9 @@ import { getOriginalWaitUntil } from '../flush'; * * 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. + * owns the invocation — including detached continuations created inside it. It lives in + * the scope's `refs`, so isolation scopes forked inside the invocation (`withMonitor`, + * `withIsolationScope`) share it, and it stays out of serialized event data. */ export interface InvocationState { /** @@ -40,7 +41,7 @@ export interface InvocationState { waitUntil?: ExecutionContext['waitUntil'] | null; } -const INVOCATION_STATE: unique symbol = Symbol('sentryInvocationState'); +const SCOPE_INVOCATION_FIELD = 'invocationState'; /** * Returns the invocation's original (un-instrumented) `waitUntil`, bound to its context, @@ -63,10 +64,6 @@ export function getInvocationWaitUntil(state: InvocationState): ExecutionContext return state.waitUntil ?? undefined; } -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 @@ -81,7 +78,7 @@ export function setInvocationState(scope: Scope, state: InvocationState): void { return; } - (scope as ScopeWithInvocationState)[INVOCATION_STATE] = state; + scope.refs[SCOPE_INVOCATION_FIELD] = state; } /** @@ -91,9 +88,9 @@ export function setInvocationState(scope: Scope, state: InvocationState): void { * is never attached there in the first place). */ export function getInvocationState(): InvocationState | undefined { - const isolationScope = getIsolationScope() as ScopeWithInvocationState; + const isolationScope = getIsolationScope(); if (isolationScope === getDefaultIsolationScope()) { return undefined; } - return isolationScope[INVOCATION_STATE]; + return isolationScope.refs[SCOPE_INVOCATION_FIELD] as InvocationState | undefined; } diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts index 33649a188f06..92bef2de3fcf 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts @@ -478,6 +478,45 @@ describe('instrumentScheduled', () => { expect(sentItemTypes.filter(type => type === 'check_in')).toHaveLength(2); }); + test('holds the check-ins of withMonitor for the flush at the end of the invocation', async () => { + // The runtime cancels a send that is not registered with `waitUntil` once the event ends. + // `withMonitor` forks the isolation scope, so its check-ins must still ride the boundary flush. + const sentItemTypes: string[] = []; + const transportFlush = vi.fn(async () => true); + let flushesInHandler = -1; + const wrappedHandler = withSentry( + env => ({ + dsn: env.SENTRY_DSN, + transport: () => ({ + send: async envelope => { + sentItemTypes.push(...envelope[1].map(([itemHeader]) => itemHeader.type)); + return {}; + }, + flush: transportFlush, + }), + }), + { + async scheduled() { + await SentryCore.withMonitor('foo', async () => { + await new Promise(resolve => setTimeout(resolve, 1)); + }); + flushesInHandler = transportFlush.mock.calls.length; + }, + } satisfies ExportedHandler, + ); + + const waits: Promise[] = []; + await wrappedHandler.scheduled?.(controllerFor('30 9 * * 1-5'), MOCK_ENV, { + waitUntil: vi.fn(promise => waits.push(promise)), + passThroughOnException: vi.fn(), + } as unknown as ExecutionContext); + await Promise.all(waits); + + expect(sentItemTypes.filter(type => type === 'check_in')).toHaveLength(2); + expect(flushesInHandler).toBe(0); + expect(transportFlush).toHaveBeenCalledTimes(1); + }); + test('drains the transport for the in_progress check-in while the handler runs', async () => { const transportFlush = vi.fn(async () => true); let finishHandler: () => void = () => undefined; diff --git a/packages/cloudflare/test/utils/invocationContext.test.ts b/packages/cloudflare/test/utils/invocationContext.test.ts index cd05ebf6b8cd..3378f75dfdfb 100644 --- a/packages/cloudflare/test/utils/invocationContext.test.ts +++ b/packages/cloudflare/test/utils/invocationContext.test.ts @@ -48,13 +48,16 @@ describe('invocation state', () => { expect(getInvocationState()).toBeUndefined(); }); - it('is not inherited by scope clones', () => { - const scope = getDefaultIsolationScope().clone(); - setInvocationState(scope, { ctx: undefined }); + it('is shared with isolation scopes forked inside the invocation', () => { + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; - withIsolationScope(scope.clone(), () => { - expect(getInvocationState()).toBeUndefined(); - }); + withInvocationIsolationScope(() => { + const state = getInvocationState(); + + withIsolationScope(() => { + expect(getInvocationState()).toBe(state); + }); + }, ctx); }); it('is attached at the invocation entry point and kept when reentrant', () => { From 5fbba8695f4e4a22cfd3f10451b2f7b9756f52b6 Mon Sep 17 00:00:00 2001 From: Dominic Rubas <1042243+rubas@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:47:41 +0200 Subject: [PATCH 2/4] fix(cloudflare): Send check-ins without waiting for the end of the invocation The transport holds every envelope until it is drained. Before, a check-in captured inside `withMonitor` was drained at once, because the forked scope had lost the invocation state. Now that the fork keeps the state, the `in_progress` check-in waited for the flush at the end of the invocation. For a callback that runs longer than the check-in margin, Sentry then marked the run as missed. `CloudflareClient.captureCheckIn` now drains the transport after each check-in and registers the drain with the `waitUntil` of the invocation. This replaces the drain that the scheduled handler ran only for the check-ins of Cron Triggers, so `withMonitor` and `captureCheckIn` get the same delivery. --- packages/cloudflare/src/client.ts | 16 +++++- .../worker/instrumentScheduled.ts | 5 -- .../src/integrations/cronTriggers.ts | 2 - packages/cloudflare/test/client.test.ts | 24 +++++++++ .../worker/instrumentScheduled.test.ts | 50 ++++++++++++------- 5 files changed, 71 insertions(+), 26 deletions(-) diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 3b4f591d5e22..bef4f0536d47 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -1,4 +1,4 @@ -import type { ClientOptions, Options, TracePropagationTargets } from '@sentry/core'; +import type { CheckIn, ClientOptions, MonitorConfig, Options, Scope, TracePropagationTargets } from '@sentry/core'; import { _INTERNAL_clearAiProviderSkips, _INTERNAL_flushLogsBuffer, @@ -163,6 +163,20 @@ export class CloudflareClient extends ServerRuntimeClient { } } + /** + * Sends the check-in at once instead of holding it for the flush at the end of the invocation. + * An `in_progress` check-in of a run that is longer than the check-in margin would otherwise + * reach Sentry too late, and Sentry would mark the run as missed. + */ + public override captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig, scope?: Scope): string { + const checkInId = super.captureCheckIn(checkIn, monitorConfig, scope); + const transport = this.getTransport(); + if (transport) { + this._registerWithInvocationWaitUntil(transport.flush(2000)); + } + return checkInId; + } + /** * Disposes of the client and releases all resources. * diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts b/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts index 8bf690d05175..8285830b73c1 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentScheduled.ts @@ -65,11 +65,6 @@ function wrapScheduledHandler( finishCheckIn = client ?.getIntegrationByName('CronTriggers') ?.startCheckIn(controller.cron); - // The transport holds envelopes until the flush after the handler, so a run that is longer than - // the check-in margin would deliver its in_progress check-in too late and show as missed. - if (finishCheckIn) { - waitUntil(Promise.resolve(client?.getTransport()?.flush(2000)).catch(() => undefined)); - } } catch (e) { DEBUG_BUILD && debug.warn('Failed to send the in_progress cron check-in', e); } diff --git a/packages/cloudflare/src/integrations/cronTriggers.ts b/packages/cloudflare/src/integrations/cronTriggers.ts index 289cfd18c849..4f49824018c1 100644 --- a/packages/cloudflare/src/integrations/cronTriggers.ts +++ b/packages/cloudflare/src/integrations/cronTriggers.ts @@ -139,8 +139,6 @@ const _cronTriggersIntegration = ((options: CronTriggersOptions = {}): CronTrigg ); } - // Check-ins are captured directly rather than through `withMonitor`, which would fork the - // isolation scope and lose the invocation state attached to it. const checkInId = captureCheckIn( { monitorSlug, status: 'in_progress' }, crontab ? { ...monitorSettings, schedule: { type: 'crontab', value: crontab } } : undefined, diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 2d7828be238d..389166411517 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -797,4 +797,28 @@ describe('CloudflareClient', () => { expect(flushSpy).not.toHaveBeenCalled(); }); }); + + describe('check-in delivery', () => { + it.each([true, false])( + 'drains the transport for a check-in before the flush point and registers it with the invocation waitUntil (cacheClient: %s)', + cacheClient => { + const flushMock = vi.fn().mockResolvedValue(true); + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + cacheClient, + transport: () => ({ send: vi.fn().mockResolvedValue({}), flush: flushMock }), + }); + client.init(); + const ctx = { waitUntil: vi.fn(), passThroughOnException: vi.fn() }; + + withInvocationIsolationScope(() => { + client.captureCheckIn({ monitorSlug: 'foo', status: 'in_progress' }); + }, ctx as never); + + expect(flushMock).toHaveBeenCalledTimes(1); + expect(ctx.waitUntil).toHaveBeenCalledTimes(1); + expect(ctx.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); + }, + ); + }); }); diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts index 92bef2de3fcf..843683be8925 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentScheduled.test.ts @@ -2,7 +2,7 @@ // Although this is not ideal, this is the best we can do until we have a better way to test cloudflare workers. import type { ExecutionContext, ScheduledController } from '@cloudflare/workers-types'; -import type { Event, Integration } from '@sentry/core'; +import type { Event, Integration, SerializedCheckIn } from '@sentry/core'; import * as SentryCore from '@sentry/core'; import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { CloudflareClient } from '../../../src/client'; @@ -478,43 +478,57 @@ describe('instrumentScheduled', () => { expect(sentItemTypes.filter(type => type === 'check_in')).toHaveLength(2); }); - test('holds the check-ins of withMonitor for the flush at the end of the invocation', async () => { - // The runtime cancels a send that is not registered with `waitUntil` once the event ends. - // `withMonitor` forks the isolation scope, so its check-ins must still ride the boundary flush. - const sentItemTypes: string[] = []; - const transportFlush = vi.fn(async () => true); - let flushesInHandler = -1; + test('sends the in_progress check-in of withMonitor while its callback runs', async () => { + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); + // Like the Cloudflare transport, this transport holds each envelope until it is drained. + const queued: string[] = []; + const delivered: string[] = []; + const waits: Promise[] = []; const wrappedHandler = withSentry( env => ({ dsn: env.SENTRY_DSN, transport: () => ({ send: async envelope => { - sentItemTypes.push(...envelope[1].map(([itemHeader]) => itemHeader.type)); + for (const [itemHeader, item] of envelope[1]) { + if (itemHeader.type === 'check_in') { + queued.push((item as SerializedCheckIn).status); + } + } return {}; }, - flush: transportFlush, + flush: async () => { + delivered.push(...queued.splice(0)); + return true; + }, }), }), { async scheduled() { - await SentryCore.withMonitor('foo', async () => { - await new Promise(resolve => setTimeout(resolve, 1)); + await SentryCore.withMonitor('foo', () => new Promise(resolve => setTimeout(resolve, 5 * 60_000)), { + schedule: { type: 'crontab', value: '30 9 * * *' }, + checkinMargin: 1, }); - flushesInHandler = transportFlush.mock.calls.length; }, } satisfies ExportedHandler, ); - const waits: Promise[] = []; - await wrappedHandler.scheduled?.(controllerFor('30 9 * * 1-5'), MOCK_ENV, { + const run = wrappedHandler.scheduled?.(controllerFor('30 9 * * 1-5'), MOCK_ENV, { waitUntil: vi.fn(promise => waits.push(promise)), passThroughOnException: vi.fn(), } as unknown as ExecutionContext); - await Promise.all(waits); - expect(sentItemTypes.filter(type => type === 'check_in')).toHaveLength(2); - expect(flushesInHandler).toBe(0); - expect(transportFlush).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(60_000); + expect(delivered).toEqual(['in_progress']); + expect(waits).toHaveLength(1); + + await vi.advanceTimersByTimeAsync(4 * 60_000); + await run; + await vi.runAllTimersAsync(); + await Promise.all(waits); + expect(delivered).toEqual(['in_progress', 'ok']); }); test('drains the transport for the in_progress check-in while the handler runs', async () => { From 8fd1b5b4be2ae91a5f94a22e83bdc95929a7d031 Mon Sep 17 00:00:00 2001 From: Dominic Rubas <1042243+rubas@users.noreply.github.com> Date: Wed, 14 Oct 2026 19:15:32 +0200 Subject: [PATCH 3/4] fix(cloudflare): Keep the transport drain from throwing on a synchronous request error `IsolatedPromiseBuffer.drain()` starts the queued requests in a loop. A request that throws synchronously, such as a custom `fetch` that throws, stopped the loop. The drain then threw into its caller, and the requests after it were lost. `CloudflareClient.captureCheckIn` now drains the transport. So such a `fetch` made `withMonitor` throw from its opening check-in, and the callback never ran. The drain now turns a synchronous throw into a rejected request and starts the other requests. --- packages/cloudflare/src/transport.ts | 16 +++++++++------ packages/cloudflare/test/client.test.ts | 23 ++++++++++++++++++++++ packages/cloudflare/test/transport.test.ts | 18 ++++++++++++++++- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts index b7f5a3181680..ee7bcce7730b 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -66,12 +66,16 @@ export class IsolatedPromiseBuffer { const drainController = new AbortController(); this.drainSignal = drainController.signal; - let tasks: PromiseLike[]; - try { - tasks = oldTaskProducers.map(taskProducer => taskProducer()); - } finally { - this.drainSignal = undefined; - } + const tasks = oldTaskProducers.map(taskProducer => { + // A request that throws synchronously, such as a custom `fetch`, must not stop the other requests + // or throw into the caller. + try { + return taskProducer(); + } catch (error) { + return Promise.reject(error); + } + }); + this.drainSignal = undefined; return new Promise(resolve => { const timer = setTimeout(() => { diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 389166411517..27d0eae75835 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -2,6 +2,7 @@ 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 { makeCloudflareTransport } from '../src/transport'; import { getInvocationState } from '../src/utils/invocationContext'; import { withInvocationIsolationScope } from '../src/utils/invocationScope'; @@ -820,5 +821,27 @@ describe('CloudflareClient', () => { expect(ctx.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); }, ); + + it('does not throw when the fetch of the transport throws synchronously', async () => { + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + transport: makeCloudflareTransport, + transportOptions: { + fetch: () => { + throw new Error('foo'); + }, + }, + }); + client.init(); + const waits: Promise[] = []; + const ctx = { waitUntil: vi.fn(promise => waits.push(promise)), passThroughOnException: vi.fn() }; + + withInvocationIsolationScope(() => { + expect(() => client.captureCheckIn({ monitorSlug: 'foo', status: 'in_progress' })).not.toThrow(); + }, ctx as never); + + expect(waits).toHaveLength(1); + await expect(Promise.all(waits)).resolves.toBeDefined(); + }); }); }); diff --git a/packages/cloudflare/test/transport.test.ts b/packages/cloudflare/test/transport.test.ts index 1750176f5591..3fa9f99f157f 100644 --- a/packages/cloudflare/test/transport.test.ts +++ b/packages/cloudflare/test/transport.test.ts @@ -1,4 +1,4 @@ -import type { EventEnvelope, EventItem } from '@sentry/core'; +import type { EventEnvelope, EventItem, TransportMakeRequestResponse } from '@sentry/core'; import { createEnvelope, SENTRY_BUFFER_FULL_ERROR, serializeEnvelope } from '@sentry/core'; import { afterAll, describe, expect, it, vi } from 'vitest'; import type { CloudflareTransportOptions } from '../src/transport'; @@ -236,6 +236,22 @@ describe('IsolatedPromiseBuffer', () => { expect(task2).toHaveBeenCalled(); }); + it('runs the other tasks of a drain when one task throws synchronously', async () => { + const ipb = new IsolatedPromiseBuffer(); + + const task1 = vi.fn((): Promise => { + throw new Error('foo'); + }); + const task2 = vi.fn(() => Promise.resolve({})); + + await ipb.add(task1); + await ipb.add(task2); + + await expect(ipb.drain()).resolves.toEqual(true); + + expect(task2).toHaveBeenCalled(); + }); + it('should allow for a custom fetch function to be passed in', async () => { const customFetch = vi.fn(async () => { return { From 96fbb00fd5cc2db825acfe5bd370e1aa70fa8633 Mon Sep 17 00:00:00 2001 From: Dominic Rubas <1042243+rubas@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:02:02 +0200 Subject: [PATCH 4/4] ref(cloudflare): Keep the symbol key for the invocation state in scope refs The invocation state keeps its `sentryInvocationState` symbol key, as before. It now sits in the scope's `refs` instead of on the scope itself. `Scope.clone()` spreads `refs`, which copies symbol keys too, so a forked isolation scope still shares the state of its invocation. --- .../cloudflare/src/utils/invocationContext.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/cloudflare/src/utils/invocationContext.ts b/packages/cloudflare/src/utils/invocationContext.ts index 5b80e76ea5e5..aa2f55482a2f 100644 --- a/packages/cloudflare/src/utils/invocationContext.ts +++ b/packages/cloudflare/src/utils/invocationContext.ts @@ -16,9 +16,9 @@ import { getOriginalWaitUntil } from '../flush'; * * 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. It lives in - * the scope's `refs`, so isolation scopes forked inside the invocation (`withMonitor`, - * `withIsolationScope`) share it, and it stays out of serialized event data. + * owns the invocation — including detached continuations created inside it. It lives under + * a symbol key in the scope's `refs`, so isolation scopes forked inside the invocation + * (`withMonitor`, `withIsolationScope`) share it, and it stays out of serialized event data. */ export interface InvocationState { /** @@ -41,7 +41,7 @@ export interface InvocationState { waitUntil?: ExecutionContext['waitUntil'] | null; } -const SCOPE_INVOCATION_FIELD = 'invocationState'; +const INVOCATION_STATE: unique symbol = Symbol('sentryInvocationState'); /** * Returns the invocation's original (un-instrumented) `waitUntil`, bound to its context, @@ -64,6 +64,10 @@ export function getInvocationWaitUntil(state: InvocationState): ExecutionContext return state.waitUntil ?? undefined; } +type ScopeRefsWithInvocationState = Scope['refs'] & { + [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 @@ -78,7 +82,7 @@ export function setInvocationState(scope: Scope, state: InvocationState): void { return; } - scope.refs[SCOPE_INVOCATION_FIELD] = state; + (scope.refs as ScopeRefsWithInvocationState)[INVOCATION_STATE] = state; } /** @@ -92,5 +96,5 @@ export function getInvocationState(): InvocationState | undefined { if (isolationScope === getDefaultIsolationScope()) { return undefined; } - return isolationScope.refs[SCOPE_INVOCATION_FIELD] as InvocationState | undefined; + return (isolationScope.refs as ScopeRefsWithInvocationState)[INVOCATION_STATE]; }