Repository navigation
feat: v1.1 — circuit breaker & smarter load balancing - #1
Merged
Merged
Conversation
…, engine test suite Adds a circuit-breaker node and scenario #8 in front of a flaky dependency: trips OPEN past an error-rate threshold, fast-fails with 503, goes half-open after a cooldown and closes once limited probes succeed. Adds power-of-two-choices and least-latency (response-time EMA) LB strategies. Fixes a real health-flag bug: intermediate nodes now decay back to healthy after an outage instead of staying marked unhealthy forever. Ships an in-repo headless test suite (bun test, 20 passing) and a version-wise ROADMAP.
Reviewer's GuideThis PR deepens the simulator’s resilience model by adding a fully instrumented circuit breaker with automatic probing and recovery, two load-balancing strategies backed by load/latency metrics, recoverable health scoring, and a headless engine test suite, with corresponding scenario/UI and documentation updates. Sequence diagram for circuit breaker fast-fail and recoverysequenceDiagram
actor Users
participant Breaker
participant Catalog as FlakyCatalog
participant Postgres
Users->>Breaker: request
alt circuit closed
Breaker->>Catalog: forward request
Catalog->>Postgres: query
Postgres-->>Catalog: result
Catalog-->>Breaker: success or failure
Breaker-->>Users: response
else circuit open
Breaker-->>Users: fail rejected with 503
end
alt error rate reaches tripsAt
Breaker->>Breaker: circuit opens
else cooldownMs elapsed
Breaker->>Breaker: circuit becomes half-open
Users->>Breaker: limited probe
Breaker->>Catalog: forward probe
Catalog-->>Breaker: successful probe
Breaker->>Breaker: circuit closes after probeLimit successes
end
State diagram for circuit breaker lifecyclestateDiagram-v2
[*] --> Closed
Closed --> Open: error rate >= tripsAt
Open --> Open: request rejected with 503
Open --> HalfOpen: cooldownMs elapsed
HalfOpen --> Open: probe fails
HalfOpen --> Closed: probeLimit probes succeed
Closed --> Closed: successful traffic
Open --> Closed: breaker-reset
Flow diagram for smarter load-balancer selectionflowchart TD
Request[Incoming request] --> Candidates[Healthy LB candidates]
Candidates --> Strategy{Selected strategy}
Strategy -->|p2c| Two[Choose two random candidates]
Two --> LessLoad[Select lower inflight load]
Strategy -->|least-latency| EMA[Compare response-time EMA]
EMA --> Lowest[Select lowest latency]
LessLoad --> Target[Forward request]
Lowest --> Target
Flow diagram for recoverable node health scoringflowchart LR
Outcome[Completed or failed traffic] --> Window[Health error window]
Window --> EMA[Update healthEma]
Idle[Idle tick] --> EMA
EMA --> Status{healthEma > 0.5}
Status -->|yes| Unhealthy[Remove from LB rotation]
Status -->|no| Healthy[Eligible for routing]
Unhealthy -->|traffic resumes and EMA decays| Healthy
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 12 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/sim/engine.ts" line_range="534" />
<code_context>
+ return this.fail("rejected", r, i);
+ }
+ if (n.circuit === "half-open") n.probeInFlight++;
+ r.breakerIdx = i;
+ return this.forwardFrom(n, r, i);
+ }
</code_context>
<issue_to_address>
**Stale calls alter breaker recovery**
When a request admitted while closed, or during an earlier half-open round, is still in flight when the breaker enters a later half-open round, `arrive` records only `breakerIdx`, and `cbRecord` checks the breaker’s current state, so stale outcomes count as probes, decrement `probeInFlight`, and can close or reopen the circuit without the current round’s probes. Callers see incorrect breaker state and traffic admission.
Record the circuit state and half-open round at admission, and have `cbRecord` update probe state only for requests admitted in the current half-open round.
Also at `src/sim/engine.ts:379-380`, `src/sim/engine.ts:676`, `src/sim/engine.ts:755-773`, `README.md:108`.
</issue_to_address>
### Comment 2
<location path="src/sim/engine.ts" line_range="847" />
<code_context>
- if (winTotal > 0) {
- const rate = n.errWin / winTotal;
- const a = rate > 0 ? Math.min(0.6, 0.15 + winTotal / 20) : 0.4;
- n.healthEma = n.healthEma * (1 - a) + rate * a;
- }
+ const rate = winTotal > 0 ? n.errWin / winTotal : 0;
</code_context>
<issue_to_address>
**Failed targets re-enter rotation**
When health checks are enabled, one target remains healthy, and another stays partitioned long enough for its idle EMA to decay, `sample` decays an idle unhealthy node’s `healthEma` and clears its unhealthy status without a successful probe. `pickLbTarget` then makes the still-failing node eligible again, and users see traffic sent to it.
Do not clear an unhealthy node’s status from idle EMA decay; require evidence of successful recovery before making it eligible for load balancing.
</issue_to_address>
### Comment 3
<location path="src/sim/engine.ts" line_range="674" />
<code_context>
n.doneWin++;
this.win.ok++;
this.pushLat(this.t - r.born);
+ this.updateRt(n, this.t - r.born);
+ if (r.breakerIdx !== undefined) this.cbRecord(this.nodes[r.breakerIdx], false);
if (n.def.kind === "db") {
</code_context>
<issue_to_address>
**Least-latency ignores observed response times**
When a least-latency balancer targets services that forward requests downstream, `complete` updates `rtEma` only for the terminal node, so `pickLbTarget` sees only the forwarding services’ initial estimates. Routing therefore does not adapt to their observed response times and load.
Update each forwarding target’s `rtEma` with the observed end-to-end response time when its request completes.
Also at `ROADMAP.md:21`, `README.md:107`, `src/sim/engine.ts:649-658`, `src/sim/engine.ts:675`.
</issue_to_address>
### Comment 4
<location path="src/sim/engine.ts" line_range="845-847" />
<code_context>
- n.healthEma = n.healthEma * (1 - a) + rate * a;
- }
+ const rate = winTotal > 0 ? n.errWin / winTotal : 0;
+ const a =
+ rate > 0 ? Math.min(0.6, 0.15 + winTotal / 20) : winTotal > 0 ? 0.4 : 0.03;
+ n.healthEma = n.healthEma * (1 - a) + rate * a;
n.unhealthy = !n.def.crashed && n.def.kind !== "client" && n.healthEma > 0.5;
n.doneWin = 0;
</code_context>
<issue_to_address>
**Recovered nodes stay unhealthy**
When an intermediate node marked unhealthy resumes forwarding requests successfully, `complete` records success only on the terminal node, leaving the intermediate node’s `winTotal` at zero in `sample`. Its health EMA then decays at only 0.03, so the node stays out of load-balancer rotation long after traffic recovers.
Record successful forwarded traffic for intermediate nodes so `sample` applies the active-traffic recovery rate.
</issue_to_address>
### Comment 5
<location path="src/sim/engine.ts" line_range="796" />
<code_context>
else if (kind === "conn") this.totals.connErrors++;
this.win[kind]++;
this.pushLat(this.t - r.born);
+ if (r.breakerIdx !== undefined) this.cbRecord(this.nodes[r.breakerIdx], true);
if (r.warm && r.cacheIdx !== undefined) {
const c = this.nodes[r.cacheIdx];
</code_context>
<issue_to_address>
**Breaker stats omit forwarded traffic**
When requests pass through a breaker to a downstream node, `arrive`, `complete`, and `fail` do not record forwarded successes or downstream errors in the breaker’s node windows. Since `sample` derives throughput and health from those windows, the canvas and Inspector show zero throughput and omit call errors, and the breaker can remain healthy during an outage.
Record forwarded successes and downstream failures in the breaker’s node counters and windows as well as its circuit history.
Also at `src/lib/GraphCanvas.svelte:168`.
</issue_to_address>
### Comment 6
<location path="src/sim/scenarios.ts" line_range="1143-1147" />
<code_context>
+ max: 0.9,
+ step: 0.05,
+ }),
+ knob({
+ id: "cooldown",
+ label: "Cooldown before probing",
+ type: "range",
+ target: { scope: "node", id: "breaker", field: "cooldownMs" },
+ def: 4000,
+ min: 500,
+ max: 20000,
+ step: 500,
+ unit: " ms",
+ }),
+ knob({
+ id: "probe-limit",
+ label: "Half-open probe limit",
+ type: "range",
+ target: { scope: "node", id: "breaker", field: "probeLimit" },
+ def: 2,
+ min: 1,
+ max: 10,
+ step: 1,
+ }),
+ knob({
+ id: "timeout",
+ label: "Client timeout",
+ type: "range",
+ target: { scope: "global", field: "timeoutMs" },
+ def: 4000,
+ min: 500,
</code_context>
<issue_to_address>
**Timeout setting leaks between scenarios**
When a user changes the circuit scenario’s timeout and then loads a scenario without a timeout knob, the new circuit scenario writes its timeout knob to the engine-wide `timeoutMs`, but `Engine.load` applies only the newly loaded scenario’s knobs and does not restore global settings. After changing this knob, switching to a scenario without a timeout control leaves that scenario using the circuit scenario’s timeout, changing when its requests fail without an available control to explain or correct it.
Restore global settings to scenario defaults on load, or explicitly define and apply timeout defaults for every scenario.
</issue_to_address>
### Comment 7
<location path="tests/engine.test.ts" line_range="138" />
<code_context>
+ const p2c = errorsFor("p2c");
+ const li = errorsFor("least-inflight");
+ expect(p2c).toBeLessThan(errorsFor("round-robin") * 0.6);
+ expect(Math.abs(p2c - li)).toBeLessThan(li * 0.6 + 4);
+ });
+
</code_context>
<issue_to_address>
**P2C alias passes**
When the `p2c` strategy is accidentally implemented as global least-inflight, the “power of two choices approximates least-inflight” test passes if the `p2c` branch instead selects the globally least-inflight candidate: line 138 then compares identical results, and line 137 still passes if that strategy beats round-robin. The test never distinguishes two-choice sampling from an alias.
Add a deterministic test that verifies P2C samples two distinct candidates and chooses the less-loaded one.
</issue_to_address>
### Comment 8
<location path="tests/engine.test.ts" line_range="147" />
<code_context>
+ e.setKnob("arrival", 900);
+ }).snapshot();
+ expect(s.totals.errPct).toBeLessThan(15);
+ expect(s.stats["weak"].queue).toBeLessThan(10);
+ });
+});
</code_context>
<issue_to_address>
**First-target routing passes**
When least-latency always selects the first target in the `lb` scenario, the “least-latency avoids the weak instance entirely” test passes if the strategy always picks the first candidate: line 147 sees no weak-node queue, and line 146 permits the roughly 11% overload from sending 900 rps to the 800-rps strong node. It does not establish that latency measurements affect routing.
Test that routing changes after inducing a response-time difference, with target order arranged so always choosing the first candidate fails.
</issue_to_address>
### Comment 9
<location path="tests/engine.test.ts" line_range="223" />
<code_context>
+ e.setKnob("breaker", false);
+ e.setKnob("link-down", true);
+ }).snapshot();
+ expect(off.stats["breaker"].rejected).toBe(0);
+ });
+});
</code_context>
<issue_to_address>
**Dropped calls pass disabled check**
When the disabled breaker drops calls without counting them as rejections, the “disabled breaker is a transparent pass-through” test passes if a disabled breaker drops every request: line 223 checks only that it did not increment `rejected`, not that calls reach the dependency or complete. The test therefore accepts a breaker that is not transparent.
Repair the link in this test and assert that calls complete while the disabled breaker rejects none.
</issue_to_address>
### Comment 10
<location path="tests/engine.test.ts" line_range="165" />
<code_context>
+ }).snapshot();
+ expect(off.p95).toBeGreaterThan(3000);
+ expect(off.timeouts).toBeGreaterThan(1000);
+ expect(on.totals.p95).toBeLessThan(off.p95 * 0.2);
+ });
+
</code_context>
<issue_to_address>
**Fast successes pass fast-fail check**
When the OPEN breaker returns fast successes rather than 503 rejections, the “a downed dependency makes calls slow; the breaker makes them fast” test passes if the OPEN breaker completes calls successfully instead of fast-failing with 503: line 165 checks only p95, so any fast response satisfies it. It never asserts that the breaker rejected calls.
Assert that the enabled run records fast rejections at the breaker, as well as the reduced p95.
</issue_to_address>
### Comment 11
<location path="tests/engine.test.ts" line_range="188" />
<code_context>
+ e.setKnob("link-down", false);
+ for (let i = 0; i < 60; i++) e.advance(250);
+ const s = e.snapshot();
+ expect(s.stats["breaker"].circuit).toBe("closed");
+ expect(s.totals.errPct).toBeLessThan(10);
+ });
</code_context>
<issue_to_address>
**Skipping probes passes recovery test**
When the breaker transitions directly from OPEN to CLOSED after cooldown, the “repairing the link lets half-open probes close it” test passes if the breaker skips half-open and closes directly after its cooldown: lines 188-189 check only the final state and error rate after 15 seconds of recovery. They do not establish that probes occurred or that their success closed the circuit.
Observe the circuit entering half-open after repair and assert it closes only after successful probes.
</issue_to_address>
### Comment 12
<location path="README.md" line_range="36" />
<code_context>
|---|---|
| Static diagrams that you just nod along to | A **live topology you can actually break** — every toggle moves real latency, error and throughput numbers |
-| Tutorials that only show the happy path | **Seven failure scenarios**: stampede, backpressure, throttling, LB skew, partition, stale replicas |
+| Tutorials that only show the happy path | **Eight failure scenarios**: stampede, backpressure, throttling, LB skew, partition, stale replicas, a tripping circuit breaker |
| "I get it" fading an hour later | **Hands-on memory** — break it, watch it recover, then re-check yourself |
| Trusting that health checks / least-inflight work | **See requests reroute around a dead instance in real time** |
</code_context>
<issue_to_address>
**Scenario count overstates failures**
The README calls the seven listed failure modes “Eight failure scenarios,” and the roadmap calls the v1.0 set “seven failure scenarios”; both counts include the healthy Playground baseline as a failure scenario, so readers get an incorrect count.
Distinguish total scenarios from failure scenarios in both descriptions.
Also at `ROADMAP.md:7`.
</issue_to_address>Sourcery assessment
Approval pending. 11 findings to address first.
Blocking findings: src/sim/engine.ts:534, src/sim/engine.ts:847, src/sim/engine.ts:674, src/sim/engine.ts:847, src/sim/engine.ts:796, and 6 more
| return this.fail("rejected", r, i); | ||
| } | ||
| if (n.circuit === "half-open") n.probeInFlight++; | ||
| r.breakerIdx = i; |
There was a problem hiding this comment.
🟠 High · Stale calls alter breaker recovery
When a request admitted while closed, or during an earlier half-open round, is still in flight when the breaker enters a later half-open round, arrive records only breakerIdx, and cbRecord checks the breaker’s current state, so stale outcomes count as probes, decrement probeInFlight, and can close or reopen the circuit without the current round’s probes. Callers see incorrect breaker state and traffic admission.
Record the circuit state and half-open round at admission, and have cbRecord update probe state only for requests admitted in the current half-open round.
Also at src/sim/engine.ts:379-380, src/sim/engine.ts:676, src/sim/engine.ts:755-773, README.md:108.
Prompt for AI agents
In `src/sim/engine.ts` at line 534:
**Stale calls alter breaker recovery**
When a request admitted while closed, or during an earlier half-open round, is still in flight when the breaker enters a later half-open round, `arrive` records only `breakerIdx`, and `cbRecord` checks the breaker’s current state, so stale outcomes count as probes, decrement `probeInFlight`, and can close or reopen the circuit without the current round’s probes. Callers see incorrect breaker state and traffic admission.
Record the circuit state and half-open round at admission, and have `cbRecord` update probe state only for requests admitted in the current half-open round.
Also at `src/sim/engine.ts:379-380`, `src/sim/engine.ts:676`, `src/sim/engine.ts:755-773`, `README.md:108`.| const rate = winTotal > 0 ? n.errWin / winTotal : 0; | ||
| const a = | ||
| rate > 0 ? Math.min(0.6, 0.15 + winTotal / 20) : winTotal > 0 ? 0.4 : 0.03; | ||
| n.healthEma = n.healthEma * (1 - a) + rate * a; |
There was a problem hiding this comment.
🟡 Medium · Failed targets re-enter rotation
When health checks are enabled, one target remains healthy, and another stays partitioned long enough for its idle EMA to decay, sample decays an idle unhealthy node’s healthEma and clears its unhealthy status without a successful probe. pickLbTarget then makes the still-failing node eligible again, and users see traffic sent to it.
Do not clear an unhealthy node’s status from idle EMA decay; require evidence of successful recovery before making it eligible for load balancing.
Prompt for AI agents
In `src/sim/engine.ts` at line 847:
**Failed targets re-enter rotation**
When health checks are enabled, one target remains healthy, and another stays partitioned long enough for its idle EMA to decay, `sample` decays an idle unhealthy node’s `healthEma` and clears its unhealthy status without a successful probe. `pickLbTarget` then makes the still-failing node eligible again, and users see traffic sent to it.
Do not clear an unhealthy node’s status from idle EMA decay; require evidence of successful recovery before making it eligible for load balancing.| n.doneWin++; | ||
| this.win.ok++; | ||
| this.pushLat(this.t - r.born); | ||
| this.updateRt(n, this.t - r.born); |
There was a problem hiding this comment.
🟠 High · Least-latency ignores observed response times
When a least-latency balancer targets services that forward requests downstream, complete updates rtEma only for the terminal node, so pickLbTarget sees only the forwarding services’ initial estimates. Routing therefore does not adapt to their observed response times and load.
Update each forwarding target’s rtEma with the observed end-to-end response time when its request completes.
Also at ROADMAP.md:21, README.md:107, src/sim/engine.ts:649-658, src/sim/engine.ts:675.
Prompt for AI agents
In `src/sim/engine.ts` at line 674:
**Least-latency ignores observed response times**
When a least-latency balancer targets services that forward requests downstream, `complete` updates `rtEma` only for the terminal node, so `pickLbTarget` sees only the forwarding services’ initial estimates. Routing therefore does not adapt to their observed response times and load.
Update each forwarding target’s `rtEma` with the observed end-to-end response time when its request completes.
Also at `ROADMAP.md:21`, `README.md:107`, `src/sim/engine.ts:649-658`, `src/sim/engine.ts:675`.
Comment on lines
+845
to
+847
| const a = | ||
| rate > 0 ? Math.min(0.6, 0.15 + winTotal / 20) : winTotal > 0 ? 0.4 : 0.03; | ||
| n.healthEma = n.healthEma * (1 - a) + rate * a; |
There was a problem hiding this comment.
🟡 Medium · Recovered nodes stay unhealthy
When an intermediate node marked unhealthy resumes forwarding requests successfully, complete records success only on the terminal node, leaving the intermediate node’s winTotal at zero in sample. Its health EMA then decays at only 0.03, so the node stays out of load-balancer rotation long after traffic recovers.
Record successful forwarded traffic for intermediate nodes so sample applies the active-traffic recovery rate.
Prompt for AI agents
In `src/sim/engine.ts` at lines 845-847:
**Recovered nodes stay unhealthy**
When an intermediate node marked unhealthy resumes forwarding requests successfully, `complete` records success only on the terminal node, leaving the intermediate node’s `winTotal` at zero in `sample`. Its health EMA then decays at only 0.03, so the node stays out of load-balancer rotation long after traffic recovers.
Record successful forwarded traffic for intermediate nodes so `sample` applies the active-traffic recovery rate.| else if (kind === "conn") this.totals.connErrors++; | ||
| this.win[kind]++; | ||
| this.pushLat(this.t - r.born); | ||
| if (r.breakerIdx !== undefined) this.cbRecord(this.nodes[r.breakerIdx], true); |
There was a problem hiding this comment.
🟠 High · Breaker stats omit forwarded traffic
When requests pass through a breaker to a downstream node, arrive, complete, and fail do not record forwarded successes or downstream errors in the breaker’s node windows. Since sample derives throughput and health from those windows, the canvas and Inspector show zero throughput and omit call errors, and the breaker can remain healthy during an outage.
Record forwarded successes and downstream failures in the breaker’s node counters and windows as well as its circuit history.
Also at src/lib/GraphCanvas.svelte:168.
Prompt for AI agents
In `src/sim/engine.ts` at line 796:
**Breaker stats omit forwarded traffic**
When requests pass through a breaker to a downstream node, `arrive`, `complete`, and `fail` do not record forwarded successes or downstream errors in the breaker’s node windows. Since `sample` derives throughput and health from those windows, the canvas and Inspector show zero throughput and omit call errors, and the breaker can remain healthy during an outage.
Record forwarded successes and downstream failures in the breaker’s node counters and windows as well as its circuit history.
Also at `src/lib/GraphCanvas.svelte:168`.| e.setKnob("arrival", 900); | ||
| }).snapshot(); | ||
| expect(s.totals.errPct).toBeLessThan(15); | ||
| expect(s.stats["weak"].queue).toBeLessThan(10); |
There was a problem hiding this comment.
🟡 Medium · First-target routing passes
When least-latency always selects the first target in the lb scenario, the “least-latency avoids the weak instance entirely” test passes if the strategy always picks the first candidate: line 147 sees no weak-node queue, and line 146 permits the roughly 11% overload from sending 900 rps to the 800-rps strong node. It does not establish that latency measurements affect routing.
Test that routing changes after inducing a response-time difference, with target order arranged so always choosing the first candidate fails.
Prompt for AI agents
In `tests/engine.test.ts` at line 147:
**First-target routing passes**
When least-latency always selects the first target in the `lb` scenario, the “least-latency avoids the weak instance entirely” test passes if the strategy always picks the first candidate: line 147 sees no weak-node queue, and line 146 permits the roughly 11% overload from sending 900 rps to the 800-rps strong node. It does not establish that latency measurements affect routing.
Test that routing changes after inducing a response-time difference, with target order arranged so always choosing the first candidate fails.| e.setKnob("breaker", false); | ||
| e.setKnob("link-down", true); | ||
| }).snapshot(); | ||
| expect(off.stats["breaker"].rejected).toBe(0); |
There was a problem hiding this comment.
🟡 Medium · Dropped calls pass disabled check
When the disabled breaker drops calls without counting them as rejections, the “disabled breaker is a transparent pass-through” test passes if a disabled breaker drops every request: line 223 checks only that it did not increment rejected, not that calls reach the dependency or complete. The test therefore accepts a breaker that is not transparent.
Repair the link in this test and assert that calls complete while the disabled breaker rejects none.
Prompt for AI agents
In `tests/engine.test.ts` at line 223:
**Dropped calls pass disabled check**
When the disabled breaker drops calls without counting them as rejections, the “disabled breaker is a transparent pass-through” test passes if a disabled breaker drops every request: line 223 checks only that it did not increment `rejected`, not that calls reach the dependency or complete. The test therefore accepts a breaker that is not transparent.
Repair the link in this test and assert that calls complete while the disabled breaker rejects none.| }).snapshot(); | ||
| expect(off.p95).toBeGreaterThan(3000); | ||
| expect(off.timeouts).toBeGreaterThan(1000); | ||
| expect(on.totals.p95).toBeLessThan(off.p95 * 0.2); |
There was a problem hiding this comment.
🟡 Medium · Fast successes pass fast-fail check
When the OPEN breaker returns fast successes rather than 503 rejections, the “a downed dependency makes calls slow; the breaker makes them fast” test passes if the OPEN breaker completes calls successfully instead of fast-failing with 503: line 165 checks only p95, so any fast response satisfies it. It never asserts that the breaker rejected calls.
Assert that the enabled run records fast rejections at the breaker, as well as the reduced p95.
Prompt for AI agents
In `tests/engine.test.ts` at line 165:
**Fast successes pass fast-fail check**
When the OPEN breaker returns fast successes rather than 503 rejections, the “a downed dependency makes calls slow; the breaker makes them fast” test passes if the OPEN breaker completes calls successfully instead of fast-failing with 503: line 165 checks only p95, so any fast response satisfies it. It never asserts that the breaker rejected calls.
Assert that the enabled run records fast rejections at the breaker, as well as the reduced p95.| e.setKnob("link-down", false); | ||
| for (let i = 0; i < 60; i++) e.advance(250); | ||
| const s = e.snapshot(); | ||
| expect(s.stats["breaker"].circuit).toBe("closed"); |
There was a problem hiding this comment.
🟡 Medium · Skipping probes passes recovery test
When the breaker transitions directly from OPEN to CLOSED after cooldown, the “repairing the link lets half-open probes close it” test passes if the breaker skips half-open and closes directly after its cooldown: lines 188-189 check only the final state and error rate after 15 seconds of recovery. They do not establish that probes occurred or that their success closed the circuit.
Observe the circuit entering half-open after repair and assert it closes only after successful probes.
Prompt for AI agents
In `tests/engine.test.ts` at line 188:
**Skipping probes passes recovery test**
When the breaker transitions directly from OPEN to CLOSED after cooldown, the “repairing the link lets half-open probes close it” test passes if the breaker skips half-open and closes directly after its cooldown: lines 188-189 check only the final state and error rate after 15 seconds of recovery. They do not establish that probes occurred or that their success closed the circuit.
Observe the circuit entering half-open after repair and assert it closes only after successful probes.| |---|---| | ||
| | Static diagrams that you just nod along to | A **live topology you can actually break** — every toggle moves real latency, error and throughput numbers | | ||
| | Tutorials that only show the happy path | **Seven failure scenarios**: stampede, backpressure, throttling, LB skew, partition, stale replicas | | ||
| | Tutorials that only show the happy path | **Eight failure scenarios**: stampede, backpressure, throttling, LB skew, partition, stale replicas, a tripping circuit breaker | |
There was a problem hiding this comment.
⚪ Low · Scenario count overstates failures
The README calls the seven listed failure modes “Eight failure scenarios,” and the roadmap calls the v1.0 set “seven failure scenarios”; both counts include the healthy Playground baseline as a failure scenario, so readers get an incorrect count.
Distinguish total scenarios from failure scenarios in both descriptions.
Also at ROADMAP.md:7.
Prompt for AI agents
In `README.md` at line 36:
**Scenario count overstates failures**
The README calls the seven listed failure modes “Eight failure scenarios,” and the roadmap calls the v1.0 set “seven failure scenarios”; both counts include the healthy Playground baseline as a failure scenario, so readers get an incorrect count.
Distinguish total scenarios from failure scenarios in both descriptions.
Also at `ROADMAP.md:7`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's new in v1.1 — "Resilience depth"
Go deeper on failure handling instead of wider on scenarios.
Circuit breaker
breakernode kind: trips OPEN past a recent error-rate threshold, fast-fails with 503, goes half-open after a cooldown, and closes once its limited probes succeed.Link downtoggle, flakiness knob, and breaker tuning (trips-at, cooldown, probe-limit, timeout).Smarter load balancing
Real bug fixed (found by the new tests)
Node health used only terminal outcomes, which intermediate nodes never accumulate — so a node hit by an outage stayed unhealthy forever even after traffic flowed again. Health now decays across idle ticks (fast with active traffic, slow when idle), so nodes genuinely recover. Adds a regression test.
Tests
bun test→ 20 passing (no browser needed, runs in CI anywhere).Docs
ROADMAP.mdwith the version-wise plan (v1.0 → v2.0).bun test, status v1.1.0.bun test/bun run check/bun run buildall pass.Summary by Sourcery
Strengthen the simulator’s resilience coverage with circuit breaking, smarter load balancing, recovering health checks, and automated engine validation.
New Features:
Bug Fixes:
Enhancements:
Build:
bun testscript.Documentation:
Tests: