Skip to content

Commit efada76

Browse files
fix(async/unstable): make circuit breaker state getter pure
1 parent 2312098 commit efada76

File tree

2 files changed

+93
-15
lines changed

2 files changed

+93
-15
lines changed

async/unstable_circuit_breaker.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,12 @@ export class CircuitBreaker<T = unknown> {
352352
}
353353

354354
/**
355-
* Current state of the circuit breaker.
355+
* Current stored state of the circuit breaker.
356+
*
357+
* Note: This returns the stored state without resolving time-based
358+
* transitions. After a cooldown expires, this may still show `"open"`
359+
* until the next {@linkcode execute} call or {@linkcode isAvailable}
360+
* check triggers the transition to `"half_open"`.
356361
*
357362
* @example Usage
358363
* ```ts
@@ -363,10 +368,10 @@ export class CircuitBreaker<T = unknown> {
363368
* assertEquals(breaker.state, "closed");
364369
* ```
365370
*
366-
* @returns The current {@linkcode CircuitState}.
371+
* @returns The stored {@linkcode CircuitState}.
367372
*/
368373
get state(): CircuitState {
369-
return this.#resolveCurrentState().state;
374+
return this.#state.state;
370375
}
371376

372377
/**
@@ -394,6 +399,10 @@ export class CircuitBreaker<T = unknown> {
394399
/**
395400
* Whether the circuit is currently allowing requests.
396401
*
402+
* Unlike {@linkcode state}, this resolves any pending time-based
403+
* transitions (e.g., `"open"` → `"half_open"` after cooldown) to ensure
404+
* the returned value reflects current availability.
405+
*
397406
* @example Usage
398407
* ```ts
399408
* import { CircuitBreaker } from "@std/async/unstable-circuit-breaker";

async/unstable_circuit_breaker_test.ts

Lines changed: 81 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,71 @@ Deno.test("CircuitBreaker transitions to half_open after cooldown", async () =>
150150
// Advance time past cooldown
151151
time.tick(1001);
152152

153+
// isAvailable resolves time-based transitions
154+
assertEquals(breaker.isAvailable, true);
153155
assertEquals(breaker.state, "half_open");
156+
});
157+
158+
Deno.test("CircuitBreaker state getter is pure and does not trigger transitions", async () => {
159+
using time = new FakeTime();
160+
161+
const transitions: Array<[CircuitState, CircuitState]> = [];
162+
const breaker = new CircuitBreaker({
163+
failureThreshold: 1,
164+
cooldownMs: 1000,
165+
onStateChange: (from, to) => transitions.push([from, to]),
166+
});
167+
168+
// Open the circuit
169+
try {
170+
await breaker.execute(() => Promise.reject(new Error("fail")));
171+
} catch { /* expected */ }
172+
assertEquals(breaker.state, "open");
173+
assertEquals(transitions, [["closed", "open"]]);
174+
175+
// Advance time past cooldown
176+
time.tick(1001);
177+
178+
// Reading state should NOT trigger transition or callbacks
179+
assertEquals(breaker.state, "open"); // Still shows "open" (stale)
180+
assertEquals(transitions.length, 1); // No new transitions
181+
182+
// Reading state multiple times should still not trigger
183+
breaker.state;
184+
breaker.state;
185+
assertEquals(transitions.length, 1);
186+
187+
// Only isAvailable triggers the transition
154188
assertEquals(breaker.isAvailable, true);
189+
assertEquals(breaker.state, "half_open");
190+
assertEquals(transitions, [["closed", "open"], ["open", "half_open"]]);
191+
});
192+
193+
Deno.test("CircuitBreaker execute() resolves stale state before checking", async () => {
194+
using time = new FakeTime();
195+
196+
const breaker = new CircuitBreaker({
197+
failureThreshold: 1,
198+
cooldownMs: 1000,
199+
successThreshold: 1,
200+
});
201+
202+
// Open the circuit
203+
try {
204+
await breaker.execute(() => Promise.reject(new Error("fail")));
205+
} catch { /* expected */ }
206+
assertEquals(breaker.state, "open");
207+
208+
// Advance time past cooldown
209+
time.tick(1001);
210+
211+
// State is stale (still shows open)
212+
assertEquals(breaker.state, "open");
213+
214+
// But execute() should resolve the transition and succeed
215+
const result = await breaker.execute(() => Promise.resolve("success"));
216+
assertEquals(result, "success");
217+
assertEquals(breaker.state, "closed"); // Now closed after successful execution
155218
});
156219

157220
Deno.test("CircuitBreaker closes from half_open after success threshold", async () => {
@@ -168,8 +231,9 @@ Deno.test("CircuitBreaker closes from half_open after success threshold", async
168231
await breaker.execute(() => Promise.reject(new Error("fail")));
169232
} catch { /* expected */ }
170233

171-
// Enter half_open
234+
// Enter half_open (isAvailable resolves the transition)
172235
time.tick(1001);
236+
assertEquals(breaker.isAvailable, true);
173237
assertEquals(breaker.state, "half_open");
174238

175239
// First success
@@ -195,8 +259,9 @@ Deno.test("CircuitBreaker reopens from half_open on failure", async () => {
195259
await breaker.execute(() => Promise.reject(new Error("fail")));
196260
} catch { /* expected */ }
197261

198-
// Enter half_open
262+
// Enter half_open (isAvailable resolves the transition)
199263
time.tick(1001);
264+
assertEquals(breaker.isAvailable, true);
200265
assertEquals(breaker.state, "half_open");
201266

202267
// Failure in half_open should reopen
@@ -358,10 +423,9 @@ Deno.test("CircuitBreaker onStateChange callback is invoked", async () => {
358423
} catch { /* expected */ }
359424
assertEquals(transitions, [["closed", "open"]]);
360425

361-
// Half-open
426+
// Half-open (isAvailable resolves the transition)
362427
time.tick(1001);
363-
// Access state to trigger transition
364-
breaker.state;
428+
breaker.isAvailable;
365429
assertEquals(transitions, [["closed", "open"], ["open", "half_open"]]);
366430

367431
// Close
@@ -455,8 +519,9 @@ Deno.test("CircuitBreaker half_open limits concurrent requests", async () => {
455519
await breaker.execute(() => Promise.reject(new Error("fail")));
456520
} catch { /* expected */ }
457521

458-
// Enter half-open
522+
// Enter half-open (isAvailable resolves the transition)
459523
time.tick(1001);
524+
assertEquals(breaker.isAvailable, true);
460525
assertEquals(breaker.state, "half_open");
461526

462527
// Start a slow request
@@ -521,7 +586,8 @@ Deno.test("CircuitBreaker with zero cooldown transitions immediately", async ()
521586
} catch { /* expected */ }
522587

523588
// Should immediately be half_open (or allow immediate transition)
524-
// Since cooldown is 0, checking state should transition
589+
// Since cooldown is 0, isAvailable resolves the transition
590+
assertEquals(breaker.isAvailable, true);
525591
assertEquals(breaker.state, "half_open");
526592

527593
// Should be able to close immediately
@@ -658,8 +724,9 @@ Deno.test("CircuitBreaker half_open failure invokes onStateChange and onOpen", a
658724
assertEquals(transitions, [["closed", "open"]]);
659725
assertEquals(openCalls, [1]);
660726

661-
// Enter half-open
727+
// Enter half-open (isAvailable resolves the transition)
662728
time.tick(1001);
729+
assertEquals(breaker.isAvailable, true);
663730
assertEquals(breaker.state, "half_open");
664731
assertEquals(transitions, [["closed", "open"], ["open", "half_open"]]);
665732

@@ -690,8 +757,9 @@ Deno.test("CircuitBreaker consecutiveSuccesses tracked in half_open getStats", a
690757
await breaker.execute(() => Promise.reject(new Error("fail")));
691758
} catch { /* expected */ }
692759

693-
// Enter half-open
760+
// Enter half-open (isAvailable resolves the transition)
694761
time.tick(1001);
762+
assertEquals(breaker.isAvailable, true);
695763
assertEquals(breaker.state, "half_open");
696764

697765
// First success
@@ -725,8 +793,9 @@ Deno.test("CircuitBreaker isResultFailure in half_open reopens circuit", async (
725793
await breaker.execute(() => Promise.reject(new Error("fail")));
726794
} catch { /* expected */ }
727795

728-
// Enter half-open
796+
// Enter half-open (isAvailable resolves the transition)
729797
time.tick(1001);
798+
assertEquals(breaker.isAvailable, true);
730799
assertEquals(breaker.state, "half_open");
731800

732801
// Result failure in half-open should reopen
@@ -750,10 +819,10 @@ Deno.test("CircuitBreaker handles multiple half_open concurrent slots", async ()
750819
await breaker.execute(() => Promise.reject(new Error("fail")));
751820
} catch { /* expected */ }
752821

753-
// Enter half-open
822+
// Enter half-open (isAvailable resolves the transition)
754823
time.tick(1001);
755-
assertEquals(breaker.state, "half_open");
756824
assertEquals(breaker.isAvailable, true);
825+
assertEquals(breaker.state, "half_open");
757826

758827
// Start two concurrent requests (should both be allowed)
759828
let resolve1: (() => void) | undefined;

0 commit comments

Comments
 (0)