Retry pure user-DO reads once across a reset - #263
Merged
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
ndisidore
force-pushed
the
nathan/chore-user-do-retry
branch
2 times, most recently
from
August 18, 2026 23:46
c356bec to
2440a15
Compare
Maximo-Guk
reviewed
Aug 19, 2026
ndisidore
force-pushed
the
nathan/chore-user-do-retry
branch
from
August 19, 2026 15:05
2440a15 to
f781bba
Compare
Maximo-Guk
approved these changes
Aug 19, 2026
Pure rename plus the four references, split out so the retry commit's diff shows the file as a move rather than a delete/create pair -- the content change is large enough that git's default 50% similarity threshold no longer detects it.
user_do.reset.surfaced volume (from #133/#162, which deliberately deferred retries) showed the one call in flight at reset time is worth recovering instead of surfacing to the browser. do-retry.ts gains retryOnDoReset(callWithFreshStub, log): exactly one retry, full jitter (a mass reset fails every in-flight call at once), and identity rethrow so the workerd flags the frontend classifier reads survive. The retry predicate is narrower than isDoResetError: durableObjectReset retries even when flagged overloaded (the incarnation is dead, so the queue that overloaded it died with it -- this is the shape production storage-timeout resets arrive in, {remote, overloaded, durableObjectReset}), while bare retryable retries only if the object isn't shedding load. The module header now spells out why those are two independent flag axes -- retryable/overloaded come from the kj exception type and describe the call, durableObjectReset is parsed from the tunneled description and describes the object -- and links the DO error-handling docs, which document the former pair but not the latter, and whose never-retry-overloaded guidance we deliberately diverge from. The replay-safety judgment lives at each call site as an explicit, greppable wrapper -- no method-name allowlist. A reset can't distinguish "never applied" from "applied, response lost", so only strictly pure reads are wrapped: the 14 read delegations in server.ts and the read sites on the fresh-stub session getters in overseer.ts (whoami, getChatContext, listModels, plus the two OverseerImpl-internal fresh-stub reads). The thunk re-evaluates the fresh-stub getter, so the second attempt reaches the new incarnation for free. Reads with hidden writes (listOutputs, listProvidedAccounts) and every write keep today's behavior; getChatContext's doc now pins that it must stay write-free. Successful retries log user_do.reset.recovered; correlated against user_do.reset.surfaced (which still fires per attempt, with operation attribution) this gives the retry win rate and the signal for wrapping more sites later.
ndisidore
force-pushed
the
nathan/chore-user-do-retry
branch
from
August 19, 2026 16:44
f781bba to
c6d1e6b
Compare
kentonv
approved these changes
Aug 19, 2026
tosaka07
added a commit
to tosaka07/cloudflare-os
that referenced
this pull request
Aug 20, 2026
* upstream/main: Retry pure user-DO reads once across a reset (cloudflare#263) Simplify MCP discovery flows (cloudflare#269) harden missing file handling during gadget export (cloudflare#264) Support large MCP portal catalogs (cloudflare#170)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#133 and #162 made user-DO resets survivable by minting a fresh stub per call, but deliberately left retries out until telemetry showed they were worth adding. The
user_do.reset.surfacedvolume has now made that case: the one call in flight at the moment the DO resets still fails all the way to the browser.This change retries that call once. A reset can't tell the caller whether the failed call was applied or not, so only calls that are safe to replay may retry. It requires an explicit
retryOnDoReset(() => this.#user.x())wrapper around strictly pure reads. The thunk re-evaluates the fresh-stub getter, so the second attempt reaches the new incarnation; errors are never re-wrapped, so the workerd flags the browser's classifier reads stay intact.Successful retries log
user_do.reset.recovered, which correlated againstuser_do.reset.surfacedgives the retry win rate and tells us whether more sites are worth wrapping.