From c4c1b3edd09bcca9b2b81ef70a666a39943ad950 Mon Sep 17 00:00:00 2001 From: Diwak4r Date: Mon, 27 Jul 2026 15:41:02 +0545 Subject: [PATCH] fix(edge-config): stale-if-error expiry check was inverted, serving stale forever The expiry condition compared cache time to (now + staleIfError * 1000), which always evaluates to true for any positive staleIfError duration. This means stale-if-error caches never expired, indefinitely serving stale content despite being well past the configured window. Fix: change condition to check Date.now() < cacheEntry.time + staleIfError * 1000, only serving stale content when the expiry window has not yet elapsed. Also applies the same correction to the network-error path (createHandleStaleIfErrorException). Added regression test: stale content should NOT be served after the window expires (advancing 11s past a 10s stale-if-error=10 window should throw). Signed-off-by: Diwak4r --- .../utils/fetch-with-cached-response.test.ts | 22 +++++++++++++++++++ .../src/utils/fetch-with-cached-response.ts | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/edge-config/src/utils/fetch-with-cached-response.test.ts b/packages/edge-config/src/utils/fetch-with-cached-response.test.ts index a87e8aaed..779258c25 100644 --- a/packages/edge-config/src/utils/fetch-with-cached-response.test.ts +++ b/packages/edge-config/src/utils/fetch-with-cached-response.test.ts @@ -178,6 +178,28 @@ describe('fetchWithCachedResponse', () => { await expect(data2.json()).resolves.toEqual({ name: 'John' }); }); + it('should NOT serve stale content after stale-if-error window expires', async () => { + fetchMock.mockResponseOnce(JSON.stringify({ name: 'John' }), { + headers: { ETag: 'abc123', 'content-type': 'application/json' }, + }); + + // First request — populate cache + await fetchWithCachedResponse('https://example.com/api/data'); + + // Advance past the 10s stale-if-error window (11 seconds) + jest.advanceTimersByTime(11000); + + // Second request: network error, but stale-if-error=10 has expired + fetchMock.mockAbortOnce(); + + // Should throw because stale-if-error window has expired + await expect( + fetchWithCachedResponse('https://example.com/api/data', { + headers: new Headers({ 'Cache-Control': 'stale-if-error=10' }), + }), + ).rejects.toThrow(); + }); + it('should respect stale-if-error on network faults', async () => { fetchMock.mockResponseOnce(JSON.stringify({ name: 'John' }), { headers: { ETag: 'abc123', 'content-type': 'application/json' }, diff --git a/packages/edge-config/src/utils/fetch-with-cached-response.ts b/packages/edge-config/src/utils/fetch-with-cached-response.ts index 136562e65..e9a9213b5 100644 --- a/packages/edge-config/src/utils/fetch-with-cached-response.ts +++ b/packages/edge-config/src/utils/fetch-with-cached-response.ts @@ -48,7 +48,7 @@ function createHandleStaleIfError( case 503: case 504: return typeof staleIfError === 'number' && - cachedResponseEntry.time < Date.now() + staleIfError * 1000 + Date.now() < cachedResponseEntry.time + staleIfError * 1000 ? createResponse(cachedResponseEntry) : response; default: @@ -69,7 +69,7 @@ function createHandleStaleIfErrorException( ): ResponseWithCachedResponse { if ( typeof staleIfError === 'number' && - cachedResponseEntry.time < Date.now() + staleIfError * 1000 + Date.now() < cachedResponseEntry.time + staleIfError * 1000 ) { return createResponse(cachedResponseEntry); }