fix(circuit-breaker): preserve recovery timeout when OPEN (#469)

[devoliverluccas]

Problem

recordFailure() was unconditionally updating this._lastFailureTime = Date.now() on every call, including when the circuit was already in OPEN state.

This caused isAllowed() to never find elapsed >= resetTimeoutMs as long as failures kept arriving — the circuit would stay stuck in OPEN indefinitely and never transition to HALF_OPEN.

Fix

Added an early return in recordFailure() when state === OPEN. Failures are still counted (for stats), but the recovery timer is preserved so the circuit correctly transitions to HALF_OPEN after the configured timeout.

// Before (bug): _lastFailureTime was always overwritten
recordFailure() {
  this._failureCount++;
  this._lastFailureTime = Date.now(); // ← reset timer even in OPEN state
  ...
}

// After (fix): skip timer reset when already OPEN
recordFailure() {
  this._failureCount++;
  if (this._state === STATE_OPEN) {
    return; // preserve recovery timeout
  }
  this._lastFailureTime = Date.now();
  ...
}

Testing

• CLOSED → failures reach threshold → OPEN ✓
• OPEN → recordFailure() called → timer NOT reset, circuit transitions to HALF_OPEN after timeout ✓
• HALF_OPEN → failure → back to OPEN ✓

Closes #469

Summary by CodeRabbit

• Bug Fixes
  • Improved circuit breaker reliability by correctly preserving the original failure timestamp when in open state, ensuring accurate recovery timing and more predictable system behavior during failure scenarios.

[vercel]

@devoliverluccas is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Walkthrough

The fix prevents the recovery timeout from being reset when failures are recorded while the circuit is already in the OPEN state, ensuring the circuit can transition to HALF_OPEN after the original timeout period regardless of subsequent failures.

Changes

Cohort / File(s)	Summary
Circuit Breaker Timeout Fix .aiox-core/core/ids/circuit-breaker.js	Added early return in recordFailure() when circuit is in STATE_OPEN to prevent updating _lastFailureTime, preserving the original recovery timeout while still incrementing failure count.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: preventing the recovery timeout from being reset when the circuit is in OPEN state, which is the core objective of the changeset.
Linked Issues check	✅ Passed	The changes directly address issue #469 by adding an early return in recordFailure() when state === STATE_OPEN, preserving the recovery timeout while still incrementing failure counts.
Out of Scope Changes check	✅ Passed	The changes are narrowly scoped to the circuit-breaker.js file and focus exclusively on the recordFailure() logic fix plus a minor formatting change (removing trailing newline).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.aiox-core/core/ids/circuit-breaker.js (1)

95-119: ⚠️ Potential issue | 🟡 Minor

Add explicit test coverage for lastFailureTime behavior in circuit-breaker state transitions.

While tests exist for state transitions, they do not explicitly verify the critical behavior change referenced in the code comment (fix #469):

• When circuit is already OPEN and recordFailure() is called, lastFailureTime must NOT be updated. Current tests check that lastFailureTime > 0 after entering OPEN state, but do not verify that subsequent failures while OPEN do not update this value.
• When circuit is HALF_OPEN and recordFailure() occurs, lastFailureTime must be updated before the state changes to OPEN. This is not currently asserted in the "re-opens on any failure" test.

Add assertions to verify both behaviors to ensure the recovery timeout mechanism works as intended.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.aiox-core/core/ids/circuit-breaker.js around lines 95 - 119, Add explicit
assertions around recordFailure() to cover lastFailureTime behavior: in the test
that exercises the STATE_OPEN path, capture the current _lastFailureTime, call
circuit.recordFailure() again, and assert that _lastFailureTime did not change
(remains equal to the captured value). In the test that exercises the
STATE_HALF_OPEN -> STATE_OPEN transition, ensure you capture time before calling
recordFailure(), call circuit.recordFailure(), and assert that _lastFailureTime
was updated (is greater than the previous value) and that the state switched to
STATE_OPEN; reference the recordFailure method and the _lastFailureTime,
STATE_OPEN and STATE_HALF_OPEN symbols when adding these assertions.

🧹 Nitpick comments (1)

.aiox-core/core/ids/circuit-breaker.js (1)

164-164: Minor: Missing newline at end of file.

The file ends without a trailing newline. Consider adding one for POSIX compliance and to avoid potential diff noise in future commits.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.aiox-core/core/ids/circuit-breaker.js at line 164, The file
.aiox-core/core/ids/circuit-breaker.js is missing a trailing newline at EOF;
open the file and add a single newline character at the end (after the final
closing brace "};") so the file ends with a newline for POSIX compliance and to
avoid diff noise in future commits.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In @.aiox-core/core/ids/circuit-breaker.js:
- Around line 95-119: Add explicit assertions around recordFailure() to cover
lastFailureTime behavior: in the test that exercises the STATE_OPEN path,
capture the current _lastFailureTime, call circuit.recordFailure() again, and
assert that _lastFailureTime did not change (remains equal to the captured
value). In the test that exercises the STATE_HALF_OPEN -> STATE_OPEN transition,
ensure you capture time before calling recordFailure(), call
circuit.recordFailure(), and assert that _lastFailureTime was updated (is
greater than the previous value) and that the state switched to STATE_OPEN;
reference the recordFailure method and the _lastFailureTime, STATE_OPEN and
STATE_HALF_OPEN symbols when adding these assertions.

---

Nitpick comments:
In @.aiox-core/core/ids/circuit-breaker.js:
- Line 164: The file .aiox-core/core/ids/circuit-breaker.js is missing a
trailing newline at EOF; open the file and add a single newline character at the
end (after the final closing brace "};") so the file ends with a newline for
POSIX compliance and to avoid diff noise in future commits.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ee227dfe-db1c-4338-95e4-96460fc4c734

📥 Commits

Reviewing files that changed from the base of the PR and between f74e3e7 and b06b9fe.

📒 Files selected for processing (1)

• .aiox-core/core/ids/circuit-breaker.js