DEV Community

Cover image for "half-open" twice is not the same state: the bug that shaped breakwater 1.0
Pedro Rogério
Pedro Rogério

Posted on

"half-open" twice is not the same state: the bug that shaped breakwater 1.0

breakwater is my resilience toolkit for Node.js — retry, circuit breaker, timeout, bulkhead, rate limiting, stale-while-open caching, all composable, with observability built in. It just hit 1.0.0, and the headline feature is the one no Node library did well: a circuit breaker whose state is shared across every instance of your service.

One instance sees the outage and trips the breaker. The others fail fast immediately, without each having to discover the same outage on their own users. When the cooldown elapses, exactly one of them probes the recovering dependency while the rest keep waiting.

That is the pitch. This post is about the two things I got wrong on the way there, because they were both the kind of wrong that looks right.

Part 1: a state machine where the same name means two different things

A circuit breaker is a tiny state machine. Closed, open, half-open. In one process you protect transitions with nothing at all — JavaScript is single-threaded, and the code between two awaits cannot be interrupted.

Share that state across N instances and every transition becomes a compare-and-set. That much I knew. So the store interface had:

transition(name, from, to): boolean   // swap only if the state is still `from`
Enter fullscreen mode Exit fullscreen mode

Atomic in Redis via a Lua script. Looks correct. It is not.

Here is the sequence that breaks it. The circuit is half-open and one instance is probing:

  1. A probe fails. The instance decides: reopen the circuit.
  2. That decision travels — a Lua round trip to Redis, a few milliseconds.
  3. In those milliseconds, another probe succeeds, reaches the majority, and closes the circuit. Traffic resumes. It fails again. The circuit reopens, waits out the cooldown, and enters half-open again.
  4. Now the first instance's swap lands. It says: "if the state is still half-open, make it open."

The state is half-open. The swap succeeds. And it is completely wrong — that decision belonged to a period that ended three transitions ago, and it just killed a recovery that had nothing to do with it.

This is the ABA problem, and it is easy to miss here because the states have names. half-open looks like an identity. It is not: it is a label that the circuit wears repeatedly, and comparing labels tells you nothing about whether you are still in the world you made your decision in.

My first fix was a patch, and I knew it

I noticed the race before I had a distributed store — the in-process breaker had the same window whenever a custom store was async. So I patched it: after the swap succeeded, check whether the period had flipped, and if it had, swap the state back.

I even wrote the honest comment:

// The half-open period this failure belonged to ended while the CAS
// travelled: the trip landed on a FRESH period. Hand the state back
// — best effort until stores can fence the CAS with a generation.
Enter fullscreen mode Exit fullscreen mode

"Best effort" is a confession. Two swaps are not one swap: between them, another instance sees the wrong state, and the compensating swap can itself fail. I shipped it because the alternative was redesigning the store contract, and I wasn't ready.

The real fix: identify the period, not the state

The fix is a fence — a monotonic token the store mints on every successful transition:

readState(name): { state, fence, openedAt? }
compareAndSet(name, from, to, fence): { ok, snapshot }
Enter fullscreen mode Exit fullscreen mode

The swap lands only if the state is still from and nothing has transitioned since you read that fence. The stale decision from step 4 now carries fence 7 against a store holding fence 10. Redis refuses it, atomically, in the same script. No compensation. No window.

Both compensating transitions were deleted. The code got shorter, which is how you know the design got better.

Two details that fell out of it, and that I would not have thought to add:

The failed swap returns where the circuit actually is. Losing a race used to cost a second round trip to find out what happened. Now the outcome carries the current snapshot — you lose the race and refresh your view in one shot.

The store owns the timing. openedAt lives in Redis, stamped from the server clock. Before that, an instance that never saw the trip started counting the cooldown from the moment it first noticed — so instances disagreed about when probing was allowed, and the one that noticed earliest probed too soon. Now they all read the same number.

Part 2: a promise that never settles is worse than an error

The whole premise of a distributed circuit breaker is that Redis is now on the path of every protected call. Which raises the obvious question: what happens when Redis is down?

I had an answer I was proud of. No method of the store ever rejects. If Redis is unreachable, the store answers from what the instance already knows and the circuit simply becomes local until Redis comes back. You lose agreement between instances, not the protection itself. A resilience library that fails when its own backend fails has the problem backwards.

I tested it thoroughly. Store throws → contained. Store rejects → contained. Every method, both paths, all green.

Then a reviewer asked what happens when the client doesn't throw or resolve.

ioredis, with default options — the exact configuration my own documentation recommended — has enableOfflineQueue: true. When the connection is down, commands are queued, not rejected. They settle after the retry budget runs out. Measured:

readState against a dead Redis      -> settled after 73151 ms
breaker.execute() with Redis down   -> STILL PENDING after 8000 ms
Enter fullscreen mode Exit fullscreen mode

Seventy-three seconds. And it is worse than it looks, because the degradation logic never engages: the code that decides "Redis is down, go local for 5 seconds" runs in the catch, and nothing ever reaches the catch. Every protected call, on every breaker sharing that store, waits.

My library, whose entire job is to stop a dying dependency from taking your service with it, would have taken your service down with a dying Redis. Not with an error — with silence, which is worse, because nothing times out and nothing gets logged.

The fix is a bound the store owns rather than inherits:

const raw = await Promise.race([
  client.runScript(script, keys, args),
  timeoutAfter(commandTimeoutMs)   // default 500ms
])
Enter fullscreen mode Exit fullscreen mode

Documenting enableOfflineQueue: false would not have been enough. The store's central promise cannot depend on wiring it does not control.

What the pre-1.0 review actually found

I do not ship a release without a review pass, and for 1.0.0 I ran four — core concurrency, the Redis adapter and its Lua, the public API and docs, and security. Between them, seven serious defects, every one reproduced by execution before I touched anything:

  • A breaker that, with a store that does not report timing, stopped protecting permanentlynextAttemptAt landed in the past and every subsequent call was admitted as a probe, forever.
  • Probe slots going negative, so more concurrent probes than configured hit a recovering dependency.
  • The 73-second stall above.
  • An isolate() kill switch — the one you use to cut off a compromised dependency — that un-isolated itself after a TTL.
  • A store taken offline by one slow command that failed after a newer command had already succeeded, proving Redis was up.
  • A float ttlMs that aborted the Lua script mid-write, leaving keys with no expiry at all.
  • An unvalidated reply that could wedge the circuit open with no way back.

Here is the part I want to underline: several of those were in code I had just written to fix other bugs. The self-un-isolating kill switch was a direct consequence of a TTL fix I had applied an hour earlier — renewing the lease on every read, which quietly undid the PERSIST that made isolation permanent.

I now treat the fix batch as its own reviewable unit, because that is where a meaningful share of my bugs live. Fixing something is when you are most confident and least careful.

And then CI found an eighth, after all 379 local checks passed. My command timeout used an unref'd timer — the kind of tidiness that seems obviously correct. In a process with nothing else pending, the event loop drains, the timer never fires, and the caller's promise never resolves. My protection against hanging had a hanging path of its own. It only shows up on a machine quiet enough to notice.

What 1.0 actually promises

A 1.0.0 that does not say what it protects is just a number. So the release also ships a versioning policy that spells out what semver covers here — and one distinction I had not seen stated elsewhere.

Most types in a library are ones you consume. Adding a field to those is a minor: your code keeps compiling.

But four interfaces exist for you to implement — the state store, the cache store, the metrics collector, the Redis client boundary. For those, the direction reverses: adding a required member is a breaking change, because your implementation suddenly no longer satisfies the interface. So on those four, new capabilities arrive as optional members, and a member only becomes required in a major.

That rule cost one member its place before the freeze. StateStore.subscribe was declared for a push-based invalidation that nothing called yet. An optional method the library never invokes is a promise you are not keeping — somebody implements it and waits for calls that never come. It came out. It returns when there is code behind it.

Try it

npm install breakwater ioredis
Enter fullscreen mode Exit fullscreen mode
import Redis from 'ioredis'
import { circuitBreaker } from 'breakwater'
import { redisStore, fromIoredis } from 'breakwater/redis'

const client = new Redis(process.env.REDIS_URL ?? 'redis://127.0.0.1:6379')
const store = redisStore({ client: fromIoredis(client) })

const payments = circuitBreaker({
  name: 'payments-api',   // the key the circuit is shared under
  stateStore: store,
  failureThreshold: 0.5,
  minimumCalls: 20
})

const receipt = await payments.execute(({ signal }) =>
  api.post('/charge', body, { signal })
)
Enter fullscreen mode Exit fullscreen mode

That is the whole integration. Composition, events, and the Prometheus and OpenTelemetry adapters work exactly as before — only where the state lives has changed.

If you are building anything that shares state across instances, the lesson worth stealing is the first one: a compare-and-set on a value that repeats is not a compare-and-set. Fence it, or you are trusting a name to be an identity.

Top comments (0)