[PR #425] [MERGED] fix(isr): KVCacheHandler.set() now awaits KV put to prevent perpetual STALE #563

Closed
opened 2026-05-06 13:08:46 +02:00 by BreizhHardware · 0 comments

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/425
Author: @james-elicx
Created: 3/10/2026
Status: Merged
Merged: 3/10/2026
Merged by: @james-elicx

Base: mainHead: fix/isr-kv-stale-regen


📝 Commits (2)

  • b260f7f fix(isr): KVCacheHandler.set() now awaits KV put to prevent perpetual STALE
  • f25a25e fix: remove unused variable flagged by lint

📊 Changes

2 files changed (+148 additions, -11 deletions)

View changed files

📝 packages/vinext/src/cloudflare/kv-cache-handler.ts (+6 -11)
📝 tests/kv-cache-handler.test.ts (+142 -0)

📄 Description

Problem

After a cache entry's TTL expires, every subsequent request returned X-Vinext-Cache: STALE and never transitioned to HIT. Background regeneration appeared to trigger but the cache was never updated.

Root cause: KVCacheHandler.set() called _putInBackground() and immediately returned Promise.resolve(). In the background regen path in app-rsc-entry.ts:

```js
await Promise.all([
__isrSet(htmlKey, ...), // resolves immediately — KV put not done
__isrSet(rscKey, ...),
]);
// renderFn() resolves here, before KV write completes
```

ctx.waitUntil(renderFn()) expired as soon as renderFn() resolved. The Cloudflare Workers runtime tore down the isolate while the kv.put() network operation was still in flight, silently killing the write. The entry stayed STALE forever.

The MISS path was not affected — it tees the HTML stream and builds an explicit __cachePromise that fully awaits the write before ctx.waitUntil can expire.

Fix

Replace the fire-and-forget _putInBackground() with _put() which returns the actual kv.put() promise. set() now returns that promise instead of Promise.resolve().

```ts
// Before
this._putInBackground(kvKey, json, opts);
return Promise.resolve(); // resolves before KV write

// After
return this._put(kvKey, json, opts); // resolves when KV write completes
```

_put() also registers with ctx.waitUntil() as belt-and-suspenders (same as before), so the Workers runtime keeps the isolate alive even if a caller doesn't await the returned promise.

The full regen chain is now properly awaited end-to-end:
ctx.waitUntil(renderFn())await Promise.all([__isrSet(...)])await handler.set(...)return this._put(...)return kv.put(...)

Tests

Adds three regression tests in tests/kv-cache-handler.test.ts under "STALE → regen → HIT lifecycle":

  1. set() resolves only after the KV put completes
  2. set() returned promise is the actual kv.put promise, not an immediately-resolved stub (uses a latch-controlled mock to prove this)
  3. Background regen waitUntil covers the actual KV write with a delayed put — regen promise does not settle until the write completes

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/425 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 3/10/2026 **Status:** ✅ Merged **Merged:** 3/10/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/isr-kv-stale-regen` --- ### 📝 Commits (2) - [`b260f7f`](https://github.com/cloudflare/vinext/commit/b260f7fd983b4ec49c7a8c3595fd5ae045ab42fd) fix(isr): KVCacheHandler.set() now awaits KV put to prevent perpetual STALE - [`f25a25e`](https://github.com/cloudflare/vinext/commit/f25a25e5a2b20b94f14102165c66cb01ceb3e7bc) fix: remove unused variable flagged by lint ### 📊 Changes **2 files changed** (+148 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/cloudflare/kv-cache-handler.ts` (+6 -11) 📝 `tests/kv-cache-handler.test.ts` (+142 -0) </details> ### 📄 Description ## Problem After a cache entry's TTL expires, every subsequent request returned `X-Vinext-Cache: STALE` and never transitioned to `HIT`. Background regeneration appeared to trigger but the cache was never updated. **Root cause:** `KVCacheHandler.set()` called `_putInBackground()` and immediately returned `Promise.resolve()`. In the background regen path in `app-rsc-entry.ts`: \`\`\`js await Promise.all([ __isrSet(htmlKey, ...), // resolves immediately — KV put not done __isrSet(rscKey, ...), ]); // renderFn() resolves here, before KV write completes \`\`\` `ctx.waitUntil(renderFn())` expired as soon as `renderFn()` resolved. The Cloudflare Workers runtime tore down the isolate while the `kv.put()` network operation was still in flight, silently killing the write. The entry stayed STALE forever. The MISS path was not affected — it tees the HTML stream and builds an explicit `__cachePromise` that fully awaits the write before `ctx.waitUntil` can expire. ## Fix Replace the fire-and-forget `_putInBackground()` with `_put()` which returns the actual `kv.put()` promise. `set()` now returns that promise instead of `Promise.resolve()`. \`\`\`ts // Before this._putInBackground(kvKey, json, opts); return Promise.resolve(); // resolves before KV write // After return this._put(kvKey, json, opts); // resolves when KV write completes \`\`\` `_put()` also registers with `ctx.waitUntil()` as belt-and-suspenders (same as before), so the Workers runtime keeps the isolate alive even if a caller doesn't `await` the returned promise. The full regen chain is now properly awaited end-to-end: `ctx.waitUntil(renderFn())` → `await Promise.all([__isrSet(...)])` → `await handler.set(...)` → `return this._put(...)` → `return kv.put(...)` ## Tests Adds three regression tests in `tests/kv-cache-handler.test.ts` under **"STALE → regen → HIT lifecycle"**: 1. `set()` resolves only after the KV put completes 2. `set()` returned promise is the actual `kv.put` promise, not an immediately-resolved stub (uses a latch-controlled mock to prove this) 3. Background regen `waitUntil` covers the actual KV write with a delayed put — regen promise does not settle until the write completes --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
BreizhHardware 2026-05-06 13:08:46 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/vinext#563
No description provided.