Skip to content

feat(cache): stale-on-error via LRU touch on bgFlights failure #1132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
igoramf wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(cache): stale-on-error via LRU touch on bgFlights failure #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion blocks/loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,14 @@ const wrapLoader = (
stats.cache.add(1, { status, loader });

bgFlights.do(request.url, callHandlerAndCache)
.catch((error) => logger.error(`loader error ${error}`));
.catch((error) => {
logger.error(`loader error ${error}`);
// Origin failed — extend LRU TTL so stale content keeps being served
// while origin recovers, instead of evicting and returning a hard error.
(cache as Cache & { touch?: (r: RequestInfo | URL) => Promise<void> })
.touch?.(request)
.catch((e) => logger.error(`loader touch error ${e}`));
Comment on lines +345 to +346
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The optional touch call is followed by a non-optional .catch, which can throw when touch is not implemented.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At blocks/loader.ts, line 345:

<comment>The optional `touch` call is followed by a non-optional `.catch`, which can throw when `touch` is not implemented.</comment>

<file context>
@@ -337,7 +337,14 @@ const wrapLoader = (
+                // Origin failed — extend LRU TTL so stale content keeps being served
+                // while origin recovers, instead of evicting and returning a hard error.
+                (cache as Cache & { touch?: (r: RequestInfo | URL) => Promise<void> })
+                  .touch?.(request)
+                  .catch((e) => logger.error(`loader touch error ${e}`));
+              });
</file context>
Suggested change
.touch?.(request)
.catch((e) => logger.error(`loader touch error ${e}`));
.touch?.(request)?.catch((e) => logger.error(`loader touch error ${e}`));
Fix with Cubic

});
Comment on lines +340 to +347
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix formatting to pass CI.

The pipeline reports deno fmt --check found formatting differences. Run deno fmt on this file to resolve the CI failure.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@blocks/loader.ts` around lines 340 - 347, The file has formatting differences
causing CI to fail; run the formatter (deno fmt) on the file containing the
loader error handling block (the code using logger.error(...) and the
cache.touch?.(request) call) to normalize spacing and line breaks so the catch
block matches project style; after formatting, re-run the formatter check to
ensure deno fmt --check passes.

⚠️ Potential issue | 🔴 Critical

TypeError when touch is undefined: optional chaining doesn't extend to .catch().

If the cache implementation doesn't have a touch method (e.g., non-LRU caches), touch?.(request) returns undefined, and then .catch(...) is invoked on undefined, throwing a TypeError.

🐛 Proposed fix: extend optional chaining to the catch call 
                 (cache as Cache & { touch?: (r: RequestInfo | URL) => Promise<void> })
                   .touch?.(request)
-                  .catch((e) => logger.error(`loader touch error ${e}`));
+                  ?.catch((e) => logger.error(`loader touch error ${e}`));
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@blocks/loader.ts` around lines 340 - 347, The current code calls (cache as
Cache & { touch?: (r: RequestInfo | URL) => Promise<void>
}).touch?.(request).catch(...), which throws when touch is undefined because
.catch is invoked on undefined; change the call so the optional chaining also
applies to .catch — i.e. call touch via (cache ...).touch?.(request)?.catch((e)
=> logger.error(`loader touch error ${e}`)) — or alternatively guard with an if
(typeof touch === 'function') before invoking — referencing the cache variable,
its touch? method, request and logger to locate and update the loader error
handling.

} else {
status = "hit";
stats.cache.add(1, { status, loader });
Expand Down
22 changes: 19 additions & 3 deletions runtime/caches/lrucache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,19 @@ const CACHE_TTL_RESOLUTION = parseInt(
const STALE_TTL_PERIOD = parseInt(
Deno.env.get("STALE_TTL_PERIOD") ?? "30000",
);
// Time-to-live in milliseconds to extend a stale cache entry when the origin fails to revalidate.
// Keeps the entry alive so subsequent requests can still be served stale while the origin recovers.
const ERROR_STALE_TTL = parseInt(
Deno.env.get("ERROR_STALE_TTL") ?? "3600000", // 1 hour
);

const cacheOptions = (cache: Cache) => (
{
max: CACHE_MAX_ITEMS,
maxSize: CACHE_MAX_SIZE,
ttlAutopurge: CACHE_TTL_AUTOPURGE,
ttlResolution: CACHE_TTL_RESOLUTION,
dispose: async (_value: boolean, key: string) => {
dispose: async (_value: number, key: string) => {
await cache.delete(key);
},
}
Expand Down Expand Up @@ -89,12 +94,23 @@ function createLruCacheStorage(cacheStorageInner: CacheStorage): CacheStorage {
if (!length || length == "0") {
return;
}
fileCache.set(cacheKey, true, {
size: parseInt(length),
const size = parseInt(length);
fileCache.set(cacheKey, size, {
size,
ttl,
});
return cacheInner.put(cacheKey, response);
},
// Extends the LRU TTL of a cache entry without modifying the underlying data or
// the expires header. Used when origin revalidation fails (stale-on-error) to keep
// the entry alive so stale content can continue to be served while origin recovers.
touch: async (request: RequestInfo | URL): Promise<void> => {
const cacheKey = await requestURLSHA1(request);
const size = fileCache.get(cacheKey);
if (size) {
fileCache.set(cacheKey, size, { size, ttl: ERROR_STALE_TTL });
}
},
});
},
);
Expand Down
Loading