Skip to content

fix(runner): stop paying two container-start retry budgets, degrade properly (DEV-2857) - #324

Merged
demtario merged 1 commit into
masterfrom
fix/DEV-2857-container-starting-degrade
Sep 9, 2026
Merged

fix(runner): stop paying two container-start retry budgets, degrade properly (DEV-2857)#324
demtario merged 1 commit into
masterfrom
fix/DEV-2857-container-starting-degrade

Conversation

@demtario

@demtario demtario commented Sep 9, 2026

Copy link
Copy Markdown
Member

Sentry DEMOS-1Z and DEMOS-20. Triaged under DEV-2852.

The ticket's premise was wrong, and the evidence changed the fix

DEV-2857 was filed as "the error message literally asks for a retry and nothing retries." That is false at the layer that matters.

@cloudflare/sandbox@0.12.3 — pinned in pnpm-lock.yaml since 2026-07-08, i.e. before the first DEMOS-1Z event — already retries. sandbox.writeFile and sandbox.mkdir both route through BaseTransport.fetchfetchWithResponseRetry with shouldRetry: r => r.status === 503. The budget is max(120_000, 30_000 + 90_000 + 30_000) = 150 000 ms, backoff 3s→30s capped, stopping when under 15s remain: roughly 7 attempts over ~135s per RPC. The string Container is starting. Please retry in a moment. exists only in the Durable Object's containerFetch catch, as a 503 body.

So the SandboxError reaching writeFiles is the exhausted end of an SDK retry loop, not a first attempt. Adding attempts 8–12 could not help a container that has already refused for 140s, and would push out a request that was already far too long.

The real bug, with a production measurement

The catch { /* dir may exist */ } after mkdir swallowed this transient after it had already burned a full ~140s budget. The first writeFile below then opened a fresh one.

The measurement comes from DEMOS-20, a companion issue the original triage missed entirely (same trace family, same first-seen minute as DEMOS-1Z's first event):

  • session_elapsed_bucket: >=120s
  • extra.sessionElapsedMs: 283 943 — a visitor waited 4 minutes 44 seconds before getting a 500.

284s ≈ two full SDK budgets, exactly what the code predicts: one per distinct directory, plus one for the first write.

The fix: zero retries added

The house has two patterns for platform transients, and the ticket picked the wrong one. preview-boot.ts is retry-then-terminal because we own the retry there. At-capacity / DEMOS-33 is recognise-degrade-report because the retry is not ours to do. DEMOS-1Z is the second family.

  1. session-lifecycle.tsCONTAINER_STARTING_PATTERN = /container is starting/i, isContainerStartingFailure, CONTAINER_STARTING_CODE, containerStartingMessage. Deliberately narrow and deliberately separate: it does not match NOT_RUNNING_PATTERN (a different, teardown-only fault), NOT_RUNNING_PATTERN is not widened, and isAtCapacityFailure stays capacity-only so a visitor never sees "we are at capacity" for a slow boot.
  2. index.ts mkdir catch — rethrow this transient. "dir may exist" is the only failure that swallow was written for; a container that never started is not that. This is the only lever we have on the wall clock: the SDK budget has a hard max(120_000, …) floor and setRetryTimeoutMs is reachable only through the DO's private client, so we cannot make an attempt fail faster — only stop paying for two. Worst case drops from ~284s to ~140s.
  3. index.ts degrade branch — 503 with an envelope, plus Sentry.captureException at level: "warning" under fingerprint ["tier2-session-container-starting"], mirroring the existing tier2-teardown-declined capture. Placed in the create handler's catch rather than inside writeFiles, so one branch covers mkdir, writeFile, startProcess and exposePort — all of which reach the container through the same containerFetch — and because closedWhileCreating() has already run there, preserving the orphan check.
  4. packages/runtime/src/container.ts — a container_starting tier in sessionStartMessage. Without it, a 503 with an envelope and an unknown code falls through to session start failed (503): …, which trips the describeRuntimeError alternation in App.tsx and tells the visitor to install Docker.

Why no retry, on the record: a "single immediate retry" compromise was considered and rejected — no mechanism to succeed against a container that has refused for 140s, and it costs a fresh 150s budget. If a future reviewer overrides this, the retry must sit at the call site (not inside writeFiles), wrap writeFiles only and never startProcess (not idempotent — it would boot a second dev server under one session), re-check isTombstoned between attempts, and take an injected sleep.

⚠️ Deploy order is load-bearing

Ship the runtime/authoring app first (the new tier is inert until the Worker sends the code), then the Worker. Reversed, visitors in the gap get the "install Docker" message.

Verification

  • pnpm test1058 tests / 1056 pass / 0 fail / 2 todo (todos pre-existing)
  • pnpm typecheck — all 4 packages clean
  • New spec pipeline/session-create-container-starting.test.mjs 5/5; session-lifecycle.test.mjs 23/23; session-start-failure.test.mjs 23/23

Revert-checks, each isolated:

Reverted Expected red Result
Edit 2 (mkdir rethrow) T2
Edit 3 (degrade branch) T1, T5
Edit 4 (message tier) the session-start-failure assertion

Plus a mutation check: widening the predicate to also match /eacces/i turns T4 red, so the narrowness is genuinely pinned rather than being an artifact of the fake sandbox.

T2 is the load-bearing test — it asserts mkdir called once and writeFile/startProcess never. That call-count assertion is the whole proof of the double-budget fix, and it stands in for the "retry count" assertion the ticket asked for, since the retry belongs to the SDK.

Two harness facts from the plan turned out to be wrong and are documented in the new spec's header: FRAMEWORK_DEV carries react-js, not bare react (which is BUILD_CONFIG-only and would 400); and a flat file map never triggers mkdir at all, since its directory resolves to CONTAINER_ROOT which writeFiles skips — so the fixture needs a nested path.

Sentry follow-through

Fixes DEMOS-1Z and DEMOS-20. Expect volume to move rather than vanish: DEMOS-1Z stops, tier2-session-container-starting starts at roughly 7/month as warning instead of error. If it drops to exactly zero, suspect the message match broke rather than that the world improved.

Noted, out of scope

instance_type: standard-1, max_instances: 5. The events cluster (4 within 6 minutes on 09-02, ~35 min after a deploy), which smells like post-deploy image cold start or pool churn rather than a per-request race. Worth its own ticket.

🤖 Generated with Claude Code


Note

Medium Risk
Changes Tier-2 session create failure handling and user-visible errors on a hot path; mitigated by narrow message matching, no added retries, and extensive pipeline tests. Deploy order (runtime before Worker) matters for correct UX.

Overview
Fixes DEV-2857: when sandbox mkdir fails with the platform’s exhausted “Container is starting” error, the Worker no longer swallows it and burns a second SDK retry budget on the following writeFile (~4+ minute sessions).

Worker: isContainerStartingFailure and a container_starting 503 envelope (user-facing copy + Sentry warning fingerprint tier2-session-container-starting). The mkdir catch rethrows this transient; the session create handler degrades instead of returning 500.

Runtime: sessionStartMessage passes through container_starting unwrapped so the authoring app does not rewrite the message into “install Docker”.

Tests: @sentry/cloudflare stub in pipeline hooks; new POST /api/session route tests (503 degrade, single mkdir, EEXIST/EACCES guards, Sentry level/fingerprint).

Deploy: Ship runtime/authoring before the Worker so the new error code is understood client-side.

Reviewed by Cursor Bugbot for commit c96008e. Bugbot is set up for automated code reviews on this repo. Configure here.

Sentry DEMOS-1Z's "Container is starting. Please retry in a moment." error
is NOT a missing retry: @cloudflare/sandbox@0.12.3 already retries every
503 for it (BaseTransport.fetch -> fetchWithResponseRetry, shouldRetry:
r => r.status === 503, budget max(120_000, 30_000+90_000+30_000) = 150s,
~7 attempts). The SandboxError writeFiles() sees is the exhausted end of
that loop, not a first attempt, so this fix adds zero retries anywhere.

The real bug was workers/api/src/index.ts's mkdir catch ("dir may exist")
swallowing that transient AFTER the SDK had already burned its full ~140s
budget, so the next writeFile opened a fresh one and paid a second budget.
Sentry DEMOS-20 measured the sum directly: sessionElapsedMs 283943 (4m44s)
for one POST /api/session.

Three edits:
- session-lifecycle.ts: narrow isContainerStartingFailure predicate/message,
  deliberately separate from isAtCapacityFailure and NOT_RUNNING_PATTERN.
- index.ts: rethrow the transient from the mkdir catch instead of swallowing
  it (the only lever on the wall clock - drops worst case from ~284s to
  ~140s), and degrade to a 503 + warning-level Sentry capture in the create
  handler's catch, alongside the existing at-capacity branch.
- packages/runtime/src/container.ts: add a container_starting tier to
  sessionStartMessage so the enveloped 503 doesn't fall through to the
  generic "session start failed (503): ..." wrapper, which trips App.tsx's
  describeRuntimeError heuristic and would tell a visitor to install Docker.

Deploy order is load-bearing: ship the runtime/authoring app before the
Worker, or visitors in the gap get the install-Docker text for a slow boot.

Fixes DEMOS-1Z and DEMOS-20.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@qunabu

qunabu commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@demtario demtario self-assigned this Sep 9, 2026
@demtario
demtario merged commit 4adf1ed into master Sep 9, 2026
6 checks passed
@demtario
demtario deleted the fix/DEV-2857-container-starting-degrade branch September 9, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants