diff --git a/__tests__/unit/services/rate-refresh-outside-render.test.ts b/__tests__/unit/services/rate-refresh-outside-render.test.ts new file mode 100644 index 000000000..a02ee4ade --- /dev/null +++ b/__tests__/unit/services/rate-refresh-outside-render.test.ts @@ -0,0 +1,85 @@ +/** + * A background rate refresh must not be attributed to the render that asked + * for it. + * + * `fetchUpstream` uses `cache: 'no-store'`, and Next tracks every fetch started + * during a render. `getCachedRateSnapshot` is explicitly the "never wait on a + * third party to paint" path — nothing awaits its refresh — but starting the + * fetch synchronously still put it inside the render's async context, and Next + * reclassified the route. Production, 2026-08-28: + * + * Error: Page changed from static to dynamic at runtime /discover, + * reason: revalidate: 0 fetch https://api.coingecko.com/... /discover + * + * So the property under test is about TIMING, not about whether the refresh + * happens: the read must return without any fetch having begun, and the fetch + * must begin afterwards. + */ + +import { + getCachedRateSnapshot, + __setSnapshotForTests, +} from '@/services/currency/rateSource.server'; + +const MINUTE = 60_000; + +describe('getCachedRateSnapshot refresh timing', () => { + let fetchMock: jest.Mock; + + beforeEach(() => { + jest.useFakeTimers(); + fetchMock = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ bitcoin: { chf: 52199, usd: 58000, eur: 55000, gbp: 47000 } }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + __setSnapshotForTests(null); + }); + + afterEach(() => { + __setSnapshotForTests(null); + jest.useRealTimers(); + }); + + it('starts no fetch synchronously when the snapshot is stale', () => { + __setSnapshotForTests({ rates: { CHF: 52199 }, fetchedAt: Date.now() - 5 * MINUTE }); + + const snap = getCachedRateSnapshot(); + + // Stale but usable: served immediately, and crucially nothing has hit the + // network yet — that is what keeps the route static. + expect(snap?.rates.CHF).toBe(52199); + expect(fetchMock).not.toHaveBeenCalled(); + + jest.runOnlyPendingTimers(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('starts no fetch synchronously when the snapshot is too old to use', () => { + __setSnapshotForTests({ rates: { CHF: 52199 }, fetchedAt: Date.now() - 60 * MINUTE }); + + expect(getCachedRateSnapshot()).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + + jest.runOnlyPendingTimers(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('warms a cold process instead of staying empty forever', () => { + // Every deploy starts here. This used to return null and schedule nothing, + // so rates stayed absent until some other caller awaited them. + expect(getCachedRateSnapshot()).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + + jest.runOnlyPendingTimers(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('serves a fresh snapshot without scheduling anything', () => { + __setSnapshotForTests({ rates: { CHF: 52199 }, fetchedAt: Date.now() }); + + expect(getCachedRateSnapshot()?.rates.CHF).toBe(52199); + jest.runOnlyPendingTimers(); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/services/currency/rateSource.server.ts b/src/services/currency/rateSource.server.ts index ed7fc0f36..29ef80be7 100644 --- a/src/services/currency/rateSource.server.ts +++ b/src/services/currency/rateSource.server.ts @@ -127,6 +127,34 @@ function refresh(): Promise { return inFlight; } +/** + * Start a refresh that does NOT belong to whatever render asked for it. + * + * `fetchUpstream` uses `cache: 'no-store'`, and Next tracks every fetch made + * during a render. A no-store fetch inside the render of a page that was + * statically prerendered raises, in production: + * + * Error: Page changed from static to dynamic at runtime /discover, + * reason: revalidate: 0 fetch https://api.coingecko.com/... /discover + * + * The refresh here is deliberately fire-and-forget — nothing awaits it and the + * caller has already returned a snapshot or null — but "not awaited" is not the + * same as "not attributed". Next sees the fetch start inside the render's async + * context and reclassifies the route, and the page that was meant to paint + * instantly instead fails its render. + * + * A macrotask boundary puts the fetch outside that context, so the refresh + * still happens, on the same schedule, without changing how the route renders. + * Deliberately not `after()` from next/server: this module is called from + * plain server code as well as from requests, and it must not require a + * request scope to exist. + */ +function scheduleRefresh(): void { + setTimeout(() => { + void refresh(); + }, 0); +} + /** * The current rates without ever touching the network. * @@ -136,15 +164,21 @@ function refresh(): Promise { */ export function getCachedRateSnapshot(): RateSnapshot | null { if (!snapshot) { + // A cold process — every deploy — had no way to warm itself from here: it + // returned null and scheduled nothing, so amounts stayed in BTC until some + // other caller happened to await getRateSnapshot(). Now that a refresh no + // longer contaminates the render that triggered it, the first reader can + // safely start one for the next. + scheduleRefresh(); return null; } const age = ageOf(snapshot); if (age > MAX_AGE_MS) { - void refresh(); + scheduleRefresh(); return null; } if (age > FRESH_MS) { - void refresh(); + scheduleRefresh(); } return snapshot; }