feat(workers): re-enable SWR on Workers via ctx.waitUntil (LAB-751) - #80
Conversation
Phase 1 (LAB-595) forced stale-while-revalidate off on Workers: workerd cancels fire-and-forget work at response return, so a background refresh died mid-flight and left its key wedged in refreshingKeys — enough wedged keys would hit maxConcurrentRefreshes and silently disable SWR cache-wide. Phase 2 threads the platform handle through instead of keeping the hard-off: - CacheRuntime gains swrRequiresWaitUntil; the Workers runtime sets it and drops the forced swrEnabled: false. Without a bound handle, wrapped reads fall back to plain (no-SWR) L1 gets — no refresh marker taken, nothing to wedge (fail-safe, not fail-wedged). - cache.withExecutionContext(ctx) returns a request-scoped view (shared L1, backend, encryptor — no new nonce counter) whose wrap/with/secure.wrap register each refresh via ctx.waitUntil. The handle is captured lexically per request because workerd ties a refresh's I/O to the request that started it; a mutable current-context slot on the singleton would be racy under concurrent requests in one isolate. - refreshingKeys hardened from Set to Map with expiring markers (SWR_REFRESH_MARKER_TTL_MS): a refresh torn down without settling (e.g. workerd dropping waitUntil work at its deadline in a warm isolate) can no longer wedge its key or permanently consume a refresh slot; an expired marker only risks a duplicate refresh, which version tokens make benign. - workerd test lane (swr.workers.test.ts, real ExecutionContexts): stale value served immediately, refresh completes under waitUntil (L1+L2), concurrent stale hits single-flight, failing refresh clears its marker, unbound reads schedule nothing and leave the key refreshable. Node semantics unchanged: swrRequiresWaitUntil unset, refreshes stay fire-and-forget (safe there), waitUntil is a no-op arg. No encryption, AAD, key-format, or wire-format change — control flow only. Co-authored-by: multica-agent <github@multica.ai>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughWorkers SWR support now binds refreshes to ChangesWorkers SWR lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FetchHandler
participant WorkersCache
participant CacheImpl
participant BackgroundRefreshManager
participant L2Backend
FetchHandler->>WorkersCache: withExecutionContext(ctx)
FetchHandler->>WorkersCache: wrap() request
WorkersCache->>CacheImpl: wrap(..., ctx.waitUntil)
CacheImpl->>BackgroundRefreshManager: scheduleRefresh(..., waitUntil)
BackgroundRefreshManager->>L2Backend: persist refreshed value
BackgroundRefreshManager-->>FetchHandler: ctx.waitUntil(refresh)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
cachekit-io/cachekit-ts#80 re-enables stale-while-revalidate on the Workers entry behind cache.withExecutionContext(ctx); update the LAB-595 footnote from 'forced off' to the phase-2 behaviour (fail-safe no-SWR without a bound context). Co-authored-by: multica-agent <github@multica.ai>
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cachekit/src/cache/background-refresh.ts`:
- Around line 107-112: Wrap the waitUntil?.(refresh) registration inside
scheduleRefresh in a try/catch so synchronous platform errors are contained and
do not propagate to the successful stale-read path in wrap. Preserve the
existing refresh promise handling and treat a registration failure as abandoning
only that refresh attempt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9d66d2b-21d5-4f0c-b658-97e76266fc21
📒 Files selected for processing (12)
packages/cachekit/README.mdpackages/cachekit/src/cache-core.tspackages/cachekit/src/cache/background-refresh.test.tspackages/cachekit/src/cache/background-refresh.tspackages/cachekit/src/constants.tspackages/cachekit/src/intents-core.tspackages/cachekit/src/l1/lru-cache.test.tspackages/cachekit/src/l1/lru-cache.tspackages/cachekit/src/workers/index.tspackages/cachekit/src/workers/runtime.tspackages/cachekit/test/workers/cache.workers.test.tspackages/cachekit/test/workers/swr.workers.test.ts
…duleRefresh waitUntil() is a synchronous platform call that can throw (e.g. a caller reusing a wrapped function across requests with a stale ExecutionContext). An uncaught throw propagated out of scheduleRefresh and failed the stale-read path in wrap() that already had a valid value to serve. Wrap the registration in try/catch so a failed registration forfeits only this refresh attempt, never the read; stranded refreshingKeys markers remain bounded by SWR_REFRESH_MARKER_TTL_MS. CodeRabbit-Resolved: background-refresh.ts:107:Guard the waitUntil registration call itself Co-authored-by: multica-agent <github@multica.ai>
1 participant
Closes LAB-751 (phase-2 follow-up to LAB-595).
What
Re-enables stale-while-revalidate on Cloudflare Workers by threading the request's
ExecutionContextinto the cache runtime, replacing the phase-1 hard-off.cache.withExecutionContext(ctx)(new, Workers surface —WorkersCache): returns a request-scoped view over the per-isolate singleton (shared L1/backend/encryptor — no new nonce counter). Functions wrapped through the view register their SWR background refreshes viactx.waitUntil, so workerd keeps them alive past response return.swrRequiresWaitUntil; wrapped reads without a bound context fall back to plain (no-SWR) L1 gets — no refresh marker is taken, so nothing can wedge. Node semantics are untouched (fire-and-forget stays safe there).refreshingKeyshardening:Set→Mapwith expiring markers (SWR_REFRESH_MARKER_TTL_MS, 60s). A refresh torn down without settling — e.g. workerd droppingwaitUntilwork at its deadline while the isolate stays warm — can no longer leave a permanent "refresh in progress" state or eat amaxConcurrentRefreshesslot forever. An expired marker only risks a duplicate refresh, which L1 version tokens already make benign.Why the per-request view (not a setter on the singleton)
workerd ties a refresh's I/O to the request that started it, and
scheduleRefreshfires synchronously inside the wrapped call — so thewaitUntilused must belong to that same request. A mutable "current ctx" slot on the singleton would be racy under concurrent requests interleaving in one isolate; a lexically captured view is correct by construction.Tests
test/workers/swr.workers.test.ts(realcreateExecutionContext/waitOnExecutionContext): stale value served immediately; refresh completes underwaitUntil(L1 + L2 both updated); N concurrent stale hits single-flight to one refresh; a failing refresh clears its marker instead of wedging; unbound reads schedule nothing and leave the key refreshable.lru-cache.test.ts),waitUntilregistration + never-rejects tests (background-refresh.test.ts).Docs
README Workers section,
/workersentry docblocks, and the existing workers smoke test updated from "SWR forced off" to the phase-2 behaviour. Companion PRs updateprotocol/sdk-feature-matrix.mdand the docs.cachekit.io Workers quickstart.Crypto/protocol gate
Not triggered: control-flow only — no encryption, AAD, key-derivation, cache-key, or ByteStorage wire change (as scoped in the issue).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation