fix: register cache writes with waitUntil so they persist on workerd#73
Merged
Merged
Conversation
void setCached() leaves the R2 put unregistered, and workerd cancels unregistered pending I/O once the response is sent — the write never lands, so the caches behind domain overview, domain keyword/page pages, and SERP research are silently re-fetched (and re-charged) on every call. Register the writes with waitUntil, matching the pattern already used in instrumentation.ts and brandLookup.ts.
6d4ac33 to
6109fce
Compare
Contributor
Contributor
|
thanks, great find! Appreciate it |
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
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.
Problem
Five cache-write sites use
void setCached(...)— a fire-and-forget promise that is never awaited and never registered withwaitUntil:DomainService.ts— domain overview (domain.rankOverview, ~100–300 credits/call) and keyword suggestionsdomainKeywordsPage.ts/domainPagesPage.ts— paginated domain keyword/page viewskeywords/services/research/serp.ts— SERP research analysisOn Cloudflare Workers, workerd cancels unregistered pending I/O once the response is sent, so the R2
putbehind these calls is dropped before it lands. The 12-hour caches these services advertise never actually persist — every identical call re-issues the full metered DataForSEO request.Easy to miss because everything looks fine functionally: results come back, the
.catchnever fires (the promise is cancelled, not rejected), and dev-mode/miniflare often lets the write finish anyway.Fix
Register the writes with
waitUntilfromcloudflare:workers, matching the pattern already used elsewhere in the codebase (instrumentation.ts,brandLookup.ts,promptExplorer.ts). Response latency is unchanged — the write still happens after the response, it just isn't cancelled anymore.No behavior change beyond the writes becoming durable. 613 tests green,
tscclean.(Found while auditing our deployment's DataForSEO spend — cache hit-rate for
get_domain_overviewwas 0% despite repeated identical calls.)Greptile Summary
This PR changes R2 cache writes so they persist after the Worker response returns. The main changes are:
waitUntil.waitUntil.waitUntil.Confidence Score: 5/5
Safe to merge with minimal risk.
The change is narrowly scoped to an established
waitUntil(setCached(...).catch(...))pattern. Cache keys, TTLs, request inputs, response schemas, and data shaping stay unchanged. No correctness or security issues were found in the changed paths.No files require special attention.
What T-Rex did
Important Files Changed
waitUntilregistration.waitUntilwhile preserving the existing cached/fetch/return flow.waitUntilwithout changing request parameters or response shaping.waitUntilso successful DataForSEO responses can persist after response completion.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Caller participant Service as Domain/SERP service participant R2 as R2 cache participant DFS as DataForSEO participant Worker as workerd waitUntil Caller->>Service: request domain/SERP data Service->>R2: getCached(cacheKey) alt cache hit R2-->>Service: cached result Service-->>Caller: return cached result else cache miss Service->>DFS: fetch metered data DFS-->>Service: API result Service->>Worker: waitUntil(setCached(cacheKey, result, ttl)) Service-->>Caller: return fresh result Worker->>R2: persist cache write after response end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Caller participant Service as Domain/SERP service participant R2 as R2 cache participant DFS as DataForSEO participant Worker as workerd waitUntil Caller->>Service: request domain/SERP data Service->>R2: getCached(cacheKey) alt cache hit R2-->>Service: cached result Service-->>Caller: return cached result else cache miss Service->>DFS: fetch metered data DFS-->>Service: API result Service->>Worker: waitUntil(setCached(cacheKey, result, ttl)) Service-->>Caller: return fresh result Worker->>R2: persist cache write after response endReviews (1): Last reviewed commit: "fix: register cache writes with waitUnti..." | Re-trigger Greptile