diff --git a/src/services/economic/index.ts b/src/services/economic/index.ts index bb8b3fa0b..5ef4a8563 100644 --- a/src/services/economic/index.ts +++ b/src/services/economic/index.ts @@ -253,7 +253,7 @@ export async function fetchBlsData(): Promise { }); } return out; - }, [] as FredSeries[]); + }, [] as FredSeries[], { shouldCache: (r) => r.length > 0 }); } export function getChangeClass(change: number | null): string { @@ -712,9 +712,9 @@ export async function fetchBisData(): Promise { try { const [policy, eer, credit] = await Promise.all([ - hPolicy?.rates?.length ? Promise.resolve(hPolicy) : bisPolicyBreaker.execute(() => client.getBisPolicyRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisPolicyFallback), - hEer?.rates?.length ? Promise.resolve(hEer) : bisEerBreaker.execute(() => client.getBisExchangeRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisEerFallback), - hCredit?.entries?.length ? Promise.resolve(hCredit) : bisCreditBreaker.execute(() => client.getBisCredit({}, { signal: AbortSignal.timeout(20_000) }), emptyBisCreditFallback), + hPolicy?.rates?.length ? Promise.resolve(hPolicy) : bisPolicyBreaker.execute(() => client.getBisPolicyRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisPolicyFallback, { shouldCache: (r) => (r.rates?.length ?? 0) > 0 }), + hEer?.rates?.length ? Promise.resolve(hEer) : bisEerBreaker.execute(() => client.getBisExchangeRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisEerFallback, { shouldCache: (r) => (r.rates?.length ?? 0) > 0 }), + hCredit?.entries?.length ? Promise.resolve(hCredit) : bisCreditBreaker.execute(() => client.getBisCredit({}, { signal: AbortSignal.timeout(20_000) }), emptyBisCreditFallback, { shouldCache: (r) => (r.entries?.length ?? 0) > 0 }), ]); return { policyRates: policy.rates ?? [], diff --git a/src/utils/circuit-breaker.ts b/src/utils/circuit-breaker.ts index e8f960ce5..b643937b0 100644 --- a/src/utils/circuit-breaker.ts +++ b/src/utils/circuit-breaker.ts @@ -330,7 +330,20 @@ export class CircuitBreaker { await this.hydratePersistentCache(cacheKey); } - const cachedEntry = this.getCacheEntry(cacheKey); + let cachedEntry = this.getCacheEntry(cacheKey); + + // If the cached data fails the shouldCache predicate, evict it and fetch + // fresh rather than serving known-invalid data for the full TTL. + // The default shouldCache (() => true) never returns false, so this only + // fires when an explicit predicate is passed. + // deletePersistentCache is fire-and-forget; on the rare case that + // hydratePersistentCache runs again before the delete commits, the entry + // is evicted once more — safe and self-resolving. + if (cachedEntry !== null && !shouldCache(cachedEntry.data as R)) { + this.evictCacheKey(cacheKey); + if (this.persistEnabled) this.deletePersistentCache(cacheKey); + cachedEntry = null; + } if (this.isStateOnCooldown()) { console.log(`[${this.name}] Currently unavailable, ${this.getCooldownRemaining()}s remaining`); diff --git a/tests/circuit-breaker-persistent-stale-ceiling.test.mts b/tests/circuit-breaker-persistent-stale-ceiling.test.mts index a32589cb8..968f93622 100644 --- a/tests/circuit-breaker-persistent-stale-ceiling.test.mts +++ b/tests/circuit-breaker-persistent-stale-ceiling.test.mts @@ -690,3 +690,97 @@ describe('CircuitBreaker — same name, different ceiling (registry behavior)', }); }); +// ============================================================ +// shouldCache eviction — cached invalid data is evicted and refetched +// ============================================================ + +describe('CircuitBreaker — shouldCache eviction of invalid cached data', () => { + const CIRCUIT_BREAKER_URL = pathToFileURL( + resolve(root, 'src/utils/circuit-breaker.ts'), + ).href; + + it('evicts cached data that fails shouldCache and fetches fresh', async () => { + const mod = await import(`${CIRCUIT_BREAKER_URL}?t=${Date.now()}-evict-fresh`); + const { createCircuitBreaker, clearAllCircuitBreakers } = mod; + clearAllCircuitBreakers(); + + try { + const breaker = createCircuitBreaker({ + name: 'ShouldCache Evict Test', + cacheTtlMs: 60 * 1000, + }); + + // Populate cache with empty array (simulates a prior failed fetch) + breaker.recordSuccess([]); + assert.deepEqual(breaker.getCached(), [], 'empty array is cached'); + + let fetchCount = 0; + const result = await breaker.execute( + async () => { fetchCount++; return [1, 2, 3]; }, + [] as number[], + { shouldCache: (r) => r.length > 0 }, + ); + + assert.equal(fetchCount, 1, 'must bypass empty cache and fetch fresh'); + assert.deepEqual(result, [1, 2, 3], 'fresh data is returned'); + assert.deepEqual(breaker.getCached(), [1, 2, 3], 'fresh data is now cached'); + } finally { + clearAllCircuitBreakers(); + } + }); + + it('serves valid cached data without fetching when shouldCache passes', async () => { + const mod = await import(`${CIRCUIT_BREAKER_URL}?t=${Date.now()}-evict-skip`); + const { createCircuitBreaker, clearAllCircuitBreakers } = mod; + clearAllCircuitBreakers(); + + try { + const breaker = createCircuitBreaker({ + name: 'ShouldCache Skip Test', + cacheTtlMs: 60 * 1000, + }); + + breaker.recordSuccess([1, 2, 3]); + + let fetchCount = 0; + const result = await breaker.execute( + async () => { fetchCount++; return [4, 5, 6]; }, + [] as number[], + { shouldCache: (r) => r.length > 0 }, + ); + + assert.equal(fetchCount, 0, 'valid cache must not trigger a live fetch'); + assert.deepEqual(result, [1, 2, 3], 'cached data is returned'); + } finally { + clearAllCircuitBreakers(); + } + }); + + it('without shouldCache, empty array is served from cache without fetching', async () => { + const mod = await import(`${CIRCUIT_BREAKER_URL}?t=${Date.now()}-evict-no-pred`); + const { createCircuitBreaker, clearAllCircuitBreakers } = mod; + clearAllCircuitBreakers(); + + try { + const breaker = createCircuitBreaker({ + name: 'No ShouldCache Test', + cacheTtlMs: 60 * 1000, + }); + + breaker.recordSuccess([]); + + let fetchCount = 0; + const result = await breaker.execute( + async () => { fetchCount++; return [1, 2, 3]; }, + [] as number[], + // No shouldCache — default is () => true, so empty array is valid + ); + + assert.equal(fetchCount, 0, 'without shouldCache, cached empty array is served'); + assert.deepEqual(result, [], 'empty cached data is returned as-is'); + } finally { + clearAllCircuitBreakers(); + } + }); +}); +