fix(metrics): a breaker state set before prom-client loads cannot overwrite a newer one (LAB-12963)

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

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<void> {}
 }
 
+/** 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<Registry['registerMetric']>[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);

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,20 +653,47 @@ 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');
     }
   }
 }
 
 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.
  */