refactor StorageCircuitBreaker.ts

Author: sosek108

lib/CircuitBreaker/AbstractCircuitBreaker.ts

@@ -0,0 +1,201 @@
+import StateMachine from '../StateMachine';
+import {CIRCUIT_BREAKER_TRANSITIONS} from './types';
+import type {CircuitBreakerOptions, CircuitBreakerState} from './types';
+
+/**
+ * Generic circuit breaker built on {@link StateMachine}.
+ *
+ * - **closed**: requests are allowed; failures are counted.
+ * - **open**: requests are rejected until {@link resetTimeoutMs} elapses.
+ * - **half-open**: the recovery-probe state. After the open timeout the breaker does NOT jump straight
+ *   back to closed (that would re-admit full load onto a still-broken dependency and immediately
+ *   re-trip — flapping). Instead it admits exactly ONE probe request: success means the dependency
+ *   recovered, so the circuit closes; failure means it's still down, so the circuit reopens for
+ *   another window. Admitting a single probe (see {@link isAllowed}) — rather than reopening the
+ *   floodgates — is also what avoids the "thundering herd" of every caller retrying at once.
+ *
+ * Subclasses implement the failure-counting policy by overriding {@link recordFailureInClosed} (and
+ * friends) — e.g. counting consecutive failures, or failures within a rolling time window, or any
+ * combination of those.
+ *
+ * @example
+ * class MyBreaker extends AbstractCircuitBreaker {
+ *     private failures = 0;
+ *     protected recordFailureInClosed() {
+ *         this.failures += 1;
+ *         return this.failures >= 3 ? `${this.failures} failures` : null;
+ *     }
+ *     protected recordSuccessInClosed() { this.failures = 0; }
+ *     protected resetFailureState() { this.failures = 0; }
+ * }
+ *
+ * const breaker = new MyBreaker({resetTimeoutMs: 30_000});
+ * if (breaker.isAllowed()) {
+ *     try {
+ *         doWork();
+ *         breaker.recordSuccess();
+ *     } catch {
+ *         breaker.recordFailure();
+ *     }
+ * }
+ */
+abstract class AbstractCircuitBreaker {
+    private machine: StateMachine<typeof CIRCUIT_BREAKER_TRANSITIONS, CircuitBreakerState>;
+
+    private openedAt = 0;
+
+    private isProbeInFlight = false;
+
+    private readonly resetTimeoutMs: number;
+
+    private readonly onTrip?: (reason: string) => void;
+
+    private readonly onClose?: () => void;
+
+    constructor(options: CircuitBreakerOptions = {}) {
+        this.resetTimeoutMs = options.resetTimeoutMs ?? 60_000;
+        this.onTrip = options.onTrip;
+        this.onClose = options.onClose;
+        this.machine = new StateMachine('closed', CIRCUIT_BREAKER_TRANSITIONS);
+    }
+
+    /** Record a failure while the circuit is closed. Returns a trip reason when the threshold is exceeded. */
+    protected abstract recordFailureInClosed(): string | null;
+
+    /** Update failure state after a successful request while the circuit is closed. */
+    protected abstract recordSuccessInClosed(): void;
+
+    /** Clear accumulated failure state without changing circuit state. */
+    protected abstract resetFailureState(): void;
+
+    /**
+     * Whether a request may proceed.
+     *
+     * Returns `false` while open. In half-open, the FIRST caller is admitted as the recovery probe and
+     * `isProbeInFlight` is latched so every subsequent caller is rejected until that probe resolves
+     * (via {@link recordSuccess} → close, or {@link recordFailure} → reopen). That single-probe gate is
+     * the whole point of half-open: it tests recovery with one request instead of letting a herd of
+     * waiting callers stampede a dependency that may still be down.
+     */
+    isAllowed(): boolean {
+        const currentState = this.getCurrentState();
+
+        if (currentState === 'open') {
+            return false;
+        }
+
+        if (currentState === 'half-open') {
+            if (this.isProbeInFlight) {
+                return false;
+            }
+            this.isProbeInFlight = true;
+        }
+
+        return true;
+    }
+
+    /**
+     * Record a failed request. May open the circuit from closed or half-open.
+     * @returns `true` when the circuit is open after recording (the request must not proceed).
+     */
+    recordFailure(): boolean {
+        if (this.machine.state === 'open') {
+            return true;
+        }
+
+        if (this.machine.state === 'half-open') {
+            this.trip();
+            return true;
+        }
+
+        const reason = this.recordFailureInClosed();
+        if (reason) {
+            this.trip(reason);
+            return true;
+        }
+
+        return false;
+    }
+
+    /** Record a successful request. Closes the circuit from half-open and clears failure counts. */
+    recordSuccess(): void {
+        if (this.machine.state === 'half-open') {
+            this.close();
+            return;
+        }
+
+        if (this.machine.state === 'closed') {
+            this.recordSuccessInClosed();
+        }
+    }
+
+    /**
+     * The current state WITHOUT advancing recovery — a pure query, safe to call without side effects.
+     * The open→half-open transition is applied only at the admission point ({@link isAllowed}); by the
+     * time a caller queries state after being admitted, that transition has already happened.
+     */
+    peekState(): CircuitBreakerState {
+        return this.machine.state;
+    }
+
+    /**
+     * Force the circuit back to closed from ANY state. This is a reset, not a transition, so it
+     * deliberately bypasses the transition graph (and does not fire {@link onClose}). Use only to wipe
+     * all state — e.g. between tests or sessions.
+     */
+    protected hardReset(): void {
+        this.machine = new StateMachine('closed', CIRCUIT_BREAKER_TRANSITIONS);
+        this.openedAt = 0;
+        this.isProbeInFlight = false;
+        this.resetFailureState();
+    }
+
+    private getCurrentState(): CircuitBreakerState {
+        this.maybeRecover();
+        return this.machine.state;
+    }
+
+    private trip(reason = ''): void {
+        if (this.machine.state === 'open') {
+            return;
+        }
+
+        this.machine = this.machine.transition('open');
+        this.openedAt = Date.now();
+        this.isProbeInFlight = false;
+        this.resetFailureState();
+        this.onTrip?.(reason);
+    }
+
+    private close(): void {
+        // close() only ever runs from half-open (see recordSuccess), and half-open → closed is the one
+        // legal closing transition — so go through transition() to keep the illegal open → closed jump
+        // an error rather than silently constructing a fresh closed machine.
+        this.machine = this.machine.transition('closed');
+        this.openedAt = 0;
+        this.isProbeInFlight = false;
+        this.resetFailureState();
+        this.onClose?.();
+    }
+
+    /**
+     * Lazily advance open → half-open once the reset timeout has elapsed. This is checked on read
+     * (via {@link getCurrentState}) rather than on a timer, so there's nothing to schedule or clean up:
+     * the transition simply becomes visible to the next caller after the window. Entering half-open
+     * clears `isProbeInFlight` so the next admitted request becomes the recovery probe.
+     */
+    private maybeRecover(): void {
+        if (this.machine.state !== 'open') {
+            return;
+        }
+
+        if (Date.now() - this.openedAt < this.resetTimeoutMs) {
+            return;
+        }
+
+        this.machine = this.machine.transition('half-open');
+        this.isProbeInFlight = false;
+    }
+}
+
+export default AbstractCircuitBreaker;

lib/CircuitBreaker/types.ts

@@ -0,0 +1,43 @@
+/**
+ * States of the circuit breaker.
+ *
+ * - **closed**: normal operation; requests flow and failures are counted.
+ * - **open**: tripped; requests are rejected outright so a known-bad dependency isn't hammered.
+ * - **half-open**: a trial state entered after the open timeout — see {@link CIRCUIT_BREAKER_TRANSITIONS}.
+ */
+type CircuitBreakerState = 'closed' | 'open' | 'half-open';
+
+/**
+ * Legal state transitions. The flow is closed → open → half-open → (closed | open).
+ *
+ * The **half-open** state exists so the breaker can test whether the dependency has recovered WITHOUT
+ * flipping straight back to fully closed. Going open → closed blindly would, on a dependency that is
+ * still down, immediately re-admit the full load and re-trip — flapping between open and closed every
+ * window. Instead, after the open timeout the breaker moves to half-open and admits a single trial
+ * ("probe") request:
+ *  - probe succeeds → the dependency is healthy again → transition to **closed** (resume normal flow).
+ *  - probe fails    → still broken → transition back to **open** for another timeout window.
+ *
+ * Admitting exactly one probe (rather than reopening the floodgates) is also what prevents the
+ * "thundering herd": many callers retrying at once the instant the timeout elapses, re-overwhelming a
+ * dependency that was just starting to recover.
+ */
+const CIRCUIT_BREAKER_TRANSITIONS = {
+    closed: ['open'],
+    open: ['half-open'],
+    'half-open': ['closed', 'open'],
+} as const satisfies Record<CircuitBreakerState, readonly CircuitBreakerState[]>;
+
+type CircuitBreakerOptions = {
+    /** Time in milliseconds the circuit stays open before moving to half-open. */
+    resetTimeoutMs?: number;
+
+    /** Called once each time the circuit opens. */
+    onTrip?: (reason: string) => void;
+
+    /** Called when the circuit closes. */
+    onClose?: () => void;
+};
+
+export type {CircuitBreakerOptions, CircuitBreakerState};
+export {CIRCUIT_BREAKER_TRANSITIONS};

lib/OnyxUtils.ts

@@ -826,10 +826,12 @@ function retryOperation<TMethod extends RetriableOnyxOperation>(
     const nextRetryAttempt = currentRetryAttempt + 1;
     const errorClass = Storage.classifyError(error);
 
-    // Once the breaker is open, every capacity write is going to fail the same way. Drop it silently —
+    // While open (or while a half-open probe is already in flight), drop capacity retries silently —
     // the breaker already emitted its single alert, and logging per failed write is exactly the storm
-    // we are suppressing. (We return before the log line below on purpose.)
-    if (errorClass === StorageErrorClass.CAPACITY && StorageCircuitBreaker.isTripped()) {
+    // we are suppressing. A rejected half-open caller is the in-flight probe failing; record that so
+    // the circuit reopens for another window. (We return before the log line below on purpose.)
+    if (errorClass === StorageErrorClass.CAPACITY && !StorageCircuitBreaker.isAllowed()) {
+        StorageCircuitBreaker.recordProbeFailure();
         return Promise.resolve();
     }
 
@@ -869,8 +871,7 @@ function retryOperation<TMethod extends RetriableOnyxOperation>(
     // cannot stop a session-wide storm — each evicted key triggers an OnyxDerived recompute that spawns
     // a fresh write with its own budget — so the breaker is what actually halts the meltdown. (The
     // already-open case returned silently at the top of this function.)
-    StorageCircuitBreaker.recordCapacityFailure();
-    if (StorageCircuitBreaker.isTripped()) {
+    if (StorageCircuitBreaker.recordCapacityFailure()) {
         // This failure tripped the breaker; it already emitted its single alert. Stop here.
         return Promise.resolve();
     }

lib/StateMachine.ts

@@ -0,0 +1,63 @@
+import type {ReadonlyDeep} from 'type-fest';
+
+/**
+ * A directed transition graph keyed by state name.
+ * Use `as const` when defining a graph so illegal transitions are caught at compile time.
+ *
+ * @example
+ * const transitions = {
+ *     idle: ['loading'],
+ *     loading: ['success', 'error'],
+ *     success: [],
+ *     error: ['idle'],
+ * } as const;
+ */
+type TransitionGraph = Readonly<Record<string, readonly string[]>>;
+
+/** Target states reachable from `Current` according to `Graph`. */
+type TransitionsFrom<Graph extends TransitionGraph, Current extends keyof Graph & string> = Graph[Current] extends ReadonlyArray<infer Target extends string> ? Target : never;
+
+/**
+ * An immutable, type-safe finite state machine.
+ * Pass the transition graph with `as const` so `transition` only accepts legal target states.
+ *
+ * @example
+ * const transitions = {
+ *     idle: ['loading'],
+ *     loading: ['success', 'error'],
+ *     success: [],
+ *     error: ['idle'],
+ * } as const;
+ *
+ * const idleMachine = new StateMachine('idle', transitions);
+ * const loadingMachine = idleMachine.transition('loading');
+ * loadingMachine.transition('success');
+ */
+class StateMachine<const Graph extends TransitionGraph, Current extends keyof Graph & string> {
+    /** The current state. Deeply readonly and owned by this state machine instance. */
+    readonly state: ReadonlyDeep<Current>;
+
+    private readonly transitions: Graph;
+
+    constructor(currentState: Current, transitions: Graph) {
+        this.state = currentState as ReadonlyDeep<Current>;
+        this.transitions = transitions;
+        Object.freeze(this);
+    }
+
+    /**
+     * Transition to a new state, returning a new state machine instance.
+     * Only transitions declared in the graph for the current state are accepted.
+     */
+    transition<Target extends TransitionsFrom<Graph, Current>>(target: Target): StateMachine<Graph, Target> {
+        const validTargets = this.transitions[this.state as Current];
+        if (!validTargets?.includes(target)) {
+            throw new Error(`Illegal transition from "${String(this.state)}" to "${String(target)}"`);
+        }
+
+        return new StateMachine(target, this.transitions);
+    }
+}
+
+export default StateMachine;
+export type {TransitionGraph, TransitionsFrom};