diff --git a/packages/cachekit/README.md b/packages/cachekit/README.md index 1fda53e..c53d952 100644 --- a/packages/cachekit/README.md +++ b/packages/cachekit/README.md @@ -893,10 +893,10 @@ last. prom-client loads in the background, starting on the event-loop turn after the cache is created, and no cache operation waits for it. Metrics recorded while it loads, including the first operations' durations, are recorded once it has -loaded. For the L1 gauges, a cache that wrote to L1 while loading publishes only -its latest L1 state, when its own load finishes. Until the next L1 write after -loading, a shared series can therefore read a cache other than the last one to -write. +loaded. For the L1 and circuit-breaker gauges, a cache that set them while +loading publishes only its latest values, when its own load finishes. Until the +next L1 write or operation after loading, a shared series can therefore read a +cache other than the last one to write. If `metrics` is enabled but `prom-client` is not installed, the SDK reports the failure once through the library logger and metrics degrade to no-ops — never diff --git a/packages/cachekit/src/cache.metrics.test.ts b/packages/cachekit/src/cache.metrics.test.ts index 05f9097..9d48fd2 100644 --- a/packages/cachekit/src/cache.metrics.test.ts +++ b/packages/cachekit/src/cache.metrics.test.ts @@ -3,6 +3,7 @@ import { Counter, Gauge, Registry } from 'prom-client'; import { createCache } from './cache.js'; import { ConfigurationError } from './errors.js'; import { CacheMetrics } from './metrics/prometheus.js'; +import { setLogger } from './logger.js'; import type { SecureCache } from './types/cache.js'; import type { Backend } from './backends/types.js'; import { InMemoryBackend, metricValue } from '../test/fixtures/metrics.js'; @@ -29,6 +30,14 @@ class FailingBackend implements Backend { async close(): Promise {} } +/** An Error whose message getter throws. */ +const unreadableError = (): Error => + Object.defineProperty(new Error(), 'message', { + get() { + throw new Error('message getter threw'); + }, + }); + describe('Cache metrics wiring', () => { const caches: SecureCache[] = []; @@ -49,6 +58,7 @@ describe('Cache metrics wiring', () => { afterEach(async () => { await Promise.all(caches.splice(0).map((c) => c.close())); + setLogger(null); }); it('records misses and get operations', async () => { @@ -293,6 +303,133 @@ describe('Cache metrics wiring', () => { expect(await metricValue(registry, 'cachekit_l1_memory_bytes')).toBe(0); }); + // The same for the circuit-breaker gauge: a state set in the turn + // prom-client finishes registering lands after every state set before load. + it('a breaker state set as prom-client finishes loading is not overwritten by earlier ones', async () => { + let setAfterLoad = false; + class ObservedRegistry extends Registry { + override registerMetric(metric: Parameters[0]): void { + super.registerMetric(metric); + if ((metric as unknown as { name: string }).name === 'cachekit_circuit_breaker_state') + queueMicrotask(() => { + metrics.setCircuitBreakerState('closed'); + setAfterLoad = true; + }); + } + } + const registry = new ObservedRegistry(); + const metrics = new CacheMetrics({ registry }); + metrics.setCircuitBreakerState('open'); + expect(registry.getSingleMetric('cachekit_circuit_breaker_state')).toBeUndefined(); + await vi.waitFor(() => expect(setAfterLoad).toBe(true)); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(await metricValue(registry, 'cachekit_circuit_breaker_state')).toBe(0); + }); + + // A breaker gauge that rejects the write (here registered by the application + // without the default label) is reported, whether initialize() publishes the + // pre-load state or a state is set after load, and leaves the load succeeded. + it('a breaker gauge error is reported and does not fail the load', async () => { + const registry = new Registry(); + new Gauge({ name: 'cachekit_circuit_breaker_state', help: 'app-owned', registers: [registry] }); + const errors: Error[] = []; + const metrics = new CacheMetrics({ + registry, + defaultLabels: { service: 'api' }, + onError: (err) => errors.push(err), + }); + metrics.setCircuitBreakerState('open'); + await vi.waitFor(() => expect(errors).toHaveLength(1)); + metrics.setCircuitBreakerState('closed'); + expect(errors).toHaveLength(2); + expect(errors.map((e) => e.message)).toEqual([ + expect.stringContaining('"service"'), + expect.stringContaining('"service"'), + ]); + await metrics.recordError('timeout'); + expect(await metricValue(registry, 'cachekit_errors_total', { error_type: 'timeout' })).toBe(1); + }); + + // String() throws on a null-prototype object, and instanceof on a Proxy whose + // getPrototypeOf trap throws, so reporting either must not. + it('a breaker gauge throwing a value no Error can wrap is reported and fails nothing', async () => { + const registry = new Registry(); + const gauge = new Gauge({ + name: 'cachekit_circuit_breaker_state', + help: 'app-owned', + registers: [registry], + }); + const thrown: unknown[] = [ + Object.create(null), + new Proxy( + {}, + { + getPrototypeOf: () => { + throw new Error('trap'); + }, + } + ), + ]; + gauge.set = () => { + throw thrown.shift(); + }; + const errors: Error[] = []; + const metrics = new CacheMetrics({ registry, onError: (err) => errors.push(err) }); + metrics.setCircuitBreakerState('open'); + await vi.waitFor(() => expect(errors).toHaveLength(1)); + expect(() => metrics.setCircuitBreakerState('closed')).not.toThrow(); + expect(thrown).toEqual([]); + expect(errors.map((e) => e.message)).toEqual([ + 'a thrown value could not be converted to an Error', + 'a thrown value could not be converted to an Error', + ]); + await metrics.recordError('timeout'); + expect(await metricValue(registry, 'cachekit_errors_total', { error_type: 'timeout' })).toBe(1); + }); + + // With no onError the library logger gets the error's message, and reading + // it can throw too, so neither report may let that escape. + it('a breaker gauge throwing an Error whose message getter throws is logged and fails nothing', async () => { + const logs: unknown[][] = []; + setLogger((message, error) => logs.push([message, error])); + const registry = new Registry(); + const gauge = new Gauge({ + name: 'cachekit_circuit_breaker_state', + help: 'app-owned', + registers: [registry], + }); + gauge.set = () => { + throw unreadableError(); + }; + const metrics = new CacheMetrics({ registry }); + metrics.setCircuitBreakerState('open'); + await vi.waitFor(() => expect(logs).toHaveLength(1)); + expect(() => metrics.setCircuitBreakerState('closed')).not.toThrow(); + expect(logs).toEqual([ + ['[cachekit] Metrics error (setCircuitBreakerState):', 'the error message could not be read'], + ['[cachekit] Metrics error (setCircuitBreakerState):', 'the error message could not be read'], + ]); + await metrics.recordError('timeout'); + expect(await metricValue(registry, 'cachekit_errors_total', { error_type: 'timeout' })).toBe(1); + }); + + it('a load failing with an Error whose message getter throws is logged once and rejects nothing', async () => { + const logs: unknown[][] = []; + setLogger((message, error) => logs.push([message, error])); + class FailingRegistry extends Registry { + override registerMetric(): void { + throw unreadableError(); + } + } + const metrics = new CacheMetrics({ registry: new FailingRegistry() }); + metrics.setCircuitBreakerState('open'); + await vi.waitFor(() => expect(logs).toHaveLength(1)); + await expect(metrics.recordMiss()).resolves.toBeUndefined(); + expect(logs).toEqual([ + [expect.stringContaining('failed to initialize'), 'the error message could not be read'], + ]); + }); + it('records hits and misses through wrap(), including the L1 SWR path', async () => { const registry = new Registry(); const cache = makeCache(registry); diff --git a/packages/cachekit/src/metrics/prometheus.ts b/packages/cachekit/src/metrics/prometheus.ts index 86f7c38..5efa7ec 100644 --- a/packages/cachekit/src/metrics/prometheus.ts +++ b/packages/cachekit/src/metrics/prometheus.ts @@ -245,6 +245,8 @@ export class CacheMetrics implements MetricsCollector { private initFailed = false; /** The latest L1 stats set before prom-client loaded, published by initialize(). */ private pendingL1: [entries: number, memoryBytes: number] | null = null; + /** The latest breaker state set before prom-client loaded, published by initialize(). */ + private pendingBreaker: 'closed' | 'open' | 'half-open' | null = null; constructor(config: MetricsConfig = {}) { this.prefix = config.prefix ?? 'cachekit'; @@ -380,10 +382,12 @@ export class CacheMetrics implements MetricsCollector { this.isReady = true; if (this.pendingL1) this.writeL1Gauges(...this.pendingL1); + if (this.pendingBreaker) this.writeBreakerGauge(this.pendingBreaker); this.pendingL1 = null; + this.pendingBreaker = null; return true; } catch (error) { - this.initError = error instanceof Error ? error : new Error(String(error)); + this.initError = toError(error); return false; } } @@ -404,7 +408,7 @@ export class CacheMetrics implements MetricsCollector { logError( '[cachekit] metrics are enabled but failed to initialize — install the optional ' + '`prom-client` peer dependency or set `metrics: false`:', - err.message + messageOf(err) ); } } @@ -433,10 +437,10 @@ export class CacheMetrics implements MetricsCollector { * Proper error handling instead of silent failures. */ private handleError(error: unknown, context: string): void { - const err = error instanceof Error ? error : new Error(String(error)); + const err = toError(error); if (!this.invokeErrorHandler(err)) { - logError(`[cachekit] Metrics error (${context}):`, err.message); + logError(`[cachekit] Metrics error (${context}):`, messageOf(err)); } } @@ -649,13 +653,19 @@ export class CacheMetrics implements MetricsCollector { */ setCircuitBreakerState(state: 'closed' | 'open' | 'half-open'): void { if (this.isReady) { - try { - this.circuitBreakerGauge?.set(this.defaultLabels, breakerValue(state)); - } catch (error) { - this.handleError(error, 'setCircuitBreakerState'); - } + this.writeBreakerGauge(state); } else if (!this.initFailed) { - void this.updateCircuitBreakerState(state); + // Before load only the latest state matters, as in setL1Stats(). + if (this.pendingBreaker === null) void this.whenReady(); + this.pendingBreaker = state; + } + } + + private writeBreakerGauge(state: 'closed' | 'open' | 'half-open'): void { + try { + this.circuitBreakerGauge?.set(this.defaultLabels, breakerValue(state)); + } catch (error) { + this.handleError(error, 'setCircuitBreakerState'); } } } @@ -663,6 +673,27 @@ export class CacheMetrics implements MetricsCollector { const breakerValue = (state: 'closed' | 'open' | 'half-open'): number => state === 'closed' ? 0 : state === 'half-open' ? 0.5 : 1; +/** Wrap a thrown value as an Error without throwing: String() throws on a + * null-prototype object, and instanceof on a Proxy whose getPrototypeOf trap + * throws. Every caller is an error path that must not escape. */ +function toError(error: unknown): Error { + try { + return error instanceof Error ? error : new Error(String(error)); + } catch { + return new Error('a thrown value could not be converted to an Error'); + } +} + +/** Read an Error's message without throwing: toError() passes an Error + * through unchanged, and its message getter can throw too. */ +function messageOf(err: Error): string { + try { + return err.message; + } catch { + return 'the error message could not be read'; + } +} + /** * No-op metrics implementation for when metrics are disabled. */