Skip to content

fix(cloudflare): Deliver check-ins and events from forked isolation scopes - #25374

Open
rubas wants to merge 3 commits into
getsentry:developfrom
rubas:fix/cloudflare-withmonitor-waituntil
Open

rubas wants to merge 3 commits into
getsentry:developfrom
rubas:fix/cloudflare-withmonitor-waituntil

Conversation

@rubas

@rubas rubas commented Oct 14, 2026

Copy link
Copy Markdown

Before

import * as Sentry from '@sentry/cloudflare';

const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));

export default Sentry.withSentry(() => ({ dsn: 'http://public@127.0.0.1:8969/1' }), {
  async scheduled() {
    await Sentry.withMonitor('foo', () => sleep(20));
  },
  async fetch() {
    await Sentry.withIsolationScope(async () => {
      await sleep(20);
      Sentry.captureMessage('bar');
    });
    return new Response('ok');
  },
} satisfies ExportedHandler);

A local ingest answers after 10 ms. It logs CANCELLED when the Worker closes the request first.

opened 16:58:18.697Z RECEIVED  check_in foo in_progress
opened 16:58:18.717Z CANCELLED at 18.719Z check_in foo ok
opened 16:58:19.346Z CANCELLED at 19.349Z event "bar"

Problem

  • withMonitor and withIsolationScope fork the isolation scope. The fork loses the invocation state.
  • Without that state, the client sends at once and does not use waitUntil. The runtime cancels the send when the handler returns.
  • So the closing check-in of withMonitor never arrives, and Sentry marks every run as timed out.

After

The fork keeps the invocation state. Each check-in leaves at once, inside waitUntil.

// invocationContext.ts: Scope.clone() copies refs, so a fork shares the state
scope.refs[SCOPE_INVOCATION_FIELD] = state;

// client.ts
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;
}

The same Worker:

opened 17:15:50.501Z RECEIVED  check_in foo in_progress
opened 17:15:50.521Z RECEIVED  check_in foo ok
opened 17:15:51.152Z RECEIVED  event "bar"

10 scenarios, 5 runs each, in scheduled, fetch, queue, a Durable Object and a Workflow step:

Envelopes develop This PR
Received 215 350
Cancelled 135 0

Fixes #25373

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
  • Link an issue if there is one related to your pull request. If no issue is linked, one will be auto-generated and linked.
Cause, design and tests

Cause. setInvocationState (utils/invocationContext.ts) stored the state under a symbol key. Scope.clone does not copy that key. In a fork, getInvocationState returned undefined, so _setupEagerEnvelopeDelivery (client.ts) sent the envelope at once without waitUntil. trpcMiddleware and wrapMcpServerWithSentry fork the scope too.

Design.

  • The state now lives in scope.refs, like the active span in spanOnScope.ts. A fork inside an invocation runs in that invocation, so it shares the state.
  • With only the refs change, the in_progress check-in waits for the flush at the end of the invocation. A run longer than the check-in margin then shows as missed. So captureCheckIn sends each check-in at once. The Cron Triggers check-in used its own drain for this before. Now all check-ins use one path, also with cacheClient: false.
  • On a cached client, the drain also sends envelopes that a concurrent invocation queued. Each send runs inside the waitUntil of the invocation with the check-in, so none is lost: 140 of 140 envelopes arrived in 20 overlapping rounds (develop: 120 of 140).
  • IsolatedPromiseBuffer.drain() now turns a synchronous throw of a request into a rejected request. Before, the throw stopped the drain and dropped the other requests.

Tests.

  • instrumentScheduled.test.ts: a 5 minute withMonitor callback sends in_progress before it ends. It fails on develop.
  • client.test.ts: captureCheckIn drains inside waitUntil, with cacheClient true and false, and does not throw when fetch throws.
  • transport.test.ts: a drain continues after a request throws.
  • invocationContext.test.ts: a forked isolation scope returns the same state.

yarn test in packages/cloudflare (1081 tests), yarn lint, yarn lint:types and yarn build:types pass. The outputs above come from wrangler dev 4.151.0.


AI disclosure: AI models helped us find this issue, write the change, and review it.

@rubas
rubas requested a review from a team as a code owner October 14, 2026 17:40
@rubas
rubas requested review from isaacs and mydea and removed request for a team October 14, 2026 17:40
@github-actions github-actions Bot added the external PR from an external contributor label Oct 14, 2026
@JPeer264
JPeer264 self-requested a review October 15, 2026 08:47
rubas added 3 commits October 15, 2026 11:11
`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.
…vocation

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.
…ous 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.
@JPeer264
JPeer264 force-pushed the fix/cloudflare-withmonitor-waituntil branch from 5752281 to 8fd1b5b Compare October 15, 2026 09:11

@JPeer264 JPeer264 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is possible to undo the symbol change then it'd be great. LGTM on the rest. Thanks a lot for creating the PR

* 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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this is now part of the client. Way cleaner than before tbh.

}

const INVOCATION_STATE: unique symbol = Symbol('sentryInvocationState');
const SCOPE_INVOCATION_FIELD = 'invocationState';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of removing the symbol tbh. We could actually leave the symbol as it was before and put it onto the .refs as you did now on line 81.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external PR from an external contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@sentry/cloudflare: The runtime cancels the closing check-in of withMonitor and events in withIsolationScope when the handler returns

2 participants