DEV Community

Cover image for My Redis library said the write succeeded. Redis was down.
Pedro Rogério
Pedro Rogério

Posted on

My Redis library said the write succeeded. Redis was down.

Last year I audited my RabbitMQ library and discovered it couldn't survive a single reconnect. I turned the whole ordeal into a playbook, and I promised myself I'd run it against the next library in my drawer.

The next library was a Redis client I wrote two years ago. The README advertised connection pooling, pub/sub, distributed locking, "battle-tested error handling" and "production-ready resilience".

Here's what the audit found: there was no pooling. There was no pub/sub. There was no locking. There were no tests. And the resilience — the one thing a Redis wrapper exists for — was worse than absent. It was actively lying.

This is the story of the second time I learned the same lesson, plus the parts that were new.

The write that succeeded by doing nothing

Every command in the library went through a health check. If Redis was unhealthy, this happened:

if (!isHealthy) {
  this.logger.warn(`Redis is not healthy. Skipping ${command} operation.`)
  return null
}
Enter fullscreen mode Exit fullscreen mode

Read that again. set, incr, lpush — writes — would resolve successfully having done nothing. No throw, no rejection. Your checkout flow calls set, gets a resolved promise, tells the user their cart is saved. The cart does not exist.

The best part? The README documented this as a feature: "Methods return null when Redis is not healthy." I had documented data loss and called it graceful degradation.

And for reads it's worse in a subtler way: get returning null means "key doesn't exist" — or now, "Redis was down". Your cache layer can't tell a miss from an outage.

The fix is a contract, not a patch:

throw new RedisClientError(
  `Redis is not connected. Cannot execute '${command}'.`,
  command,
  'REDIS_UNAVAILABLE'
)
Enter fullscreen mode Exit fullscreen mode

Structured errors with a code. The caller branches on err.code === 'REDIS_UNAVAILABLE', never on message text. A write that can't happen must be loud. This was a breaking change — and it's exactly why I fixed it before the library had public users. Defaults are free to change until the day someone depends on them.

The reconnection layer that fought the driver — and lost to itself

My library sits on top of ioredis. ioredis already reconnects. It has retryStrategy, exponential backoff, an offline queue, resubscription. It is good at this.

Two-years-ago me didn't trust it, so he built a second reconnection layer on top. Every error or close event scheduled a manual reconnect:

scheduleReconnect () {
  this.reconnectAttempts++
  setTimeout(() => this.attemptConnection(), this.reconnectInterval)
}

async attemptConnection () {
  this.client = this.redisConfig.createRedisClient()  // ← a NEW client
  // ...
}
Enter fullscreen mode Exit fullscreen mode

Spot it? Every attempt created a brand-new ioredis client without destroying the previous one. The old client — with its own retryStrategy — kept reconnecting forever in the background. Every drop leaked one more client, each running its own infinite retry loop against your server. A one-minute Redis outage turned my "resilience layer" into a connection storm.

It gets better. After the first attempt, the validation ping was skipped entirely:

if (!this.initialConnectionAttempted) {
  await this.client.ping()
  this.initialConnectionAttempted = true
}
// every reconnection after this point:
this.isConnected = true   // sure, why not
Enter fullscreen mode Exit fullscreen mode

Every reconnection "succeeded" unconditionally. isConnected = true while Redis was on fire. The reconnection had never worked and never been tested — the exact disease from the RabbitMQ library, in a different body.

And my favorite: disconnect() called quit(), which made ioredis emit close… and the close handler scheduled a reconnect. Graceful shutdown resurrected the connection. The library refused to die. There's something poetic about a resilience layer whose only reliably working feature is refusing to let you leave.

The fix was not code. It was deletion. The entire manual layer went away — the driver is the single owner of reconnection, and my library only tracks state and emits events. Four critical bugs died in one commit that removed more lines than it added.

The test that earns the word "resilient"

From the RabbitMQ refactor I kept one ritual: no reliability claim without a test that kills the connection for real, from the server side. For Redis, that's CLIENT KILL:

await client.set('sacred:before', 'ok')          // works
await killAllClientsFromTheServer()               // CLIENT KILL, server-side
await waitFor(async () =>
  await client.set('sacred:after', 'ok') === 'OK' // works again, same client
)
Enter fullscreen mode Exit fullscreen mode

Plus the two regression probes that document the old bugs forever: after recovery, the server must see exactly the same number of connections as before (no leaked reconnection loops), and after disconnect() the client must stay disconnected (no resurrection).

First run against the old code: the resurrection probe failed exactly as predicted, and the test runner never exited — leaked timers kept the process alive. The suite needed Ctrl-C. When your test suite can't die, your library can't either.

After the surgery: everything green, the runner exits by itself, and recovery after a server-side kill went from ~2 seconds to ~110 milliseconds — because reacting to the driver's events beats waiting for your own arbitrary timer.

A parade of small lies

The deep read found plenty more. A sample:

The scan that read other people's keys. getAllStream() used ioredis' scanStream — which does not apply your keyPrefix to the MATCH pattern. With a prefix configured, scanning for user:* matched zero of your own keys. And scanning * swept the entire database, including other applications' keys, which then got your prefix wrongly added on the follow-up GET and came back as null. The old code had mysterious logging for these "null values", fetching TTL and type for each one. That wasn't observability. That was a bug filing reports about itself.

maxRetryAttempts: 0 meant infinity. The classic options.x || default — zero is falsy, so "never retry" became "retry forever". Same for block: 0 in stream reads, which in Redis means "block forever" and was silently dropped. ?? exists. Use it.

XGROUP DESTROY couldn't work. The wrapper appended a default stream id to every subcommand, so the destroy documented in my own README sent a stray $ and died with a protocol error. The README showed example code that had never once executed successfully.

Every command paid a toll. The health check ran a real PING — serialized before your command, up to once per five seconds. Fail-fast checks must be local and free (client.status === 'ready'); the driver already owns the expensive part.

None of these are exotic. They're what the first test you ever write catches. There were no tests.

Then I had to earn the README

Deleting fiction from the README was easy. But two of the fictional features were genuinely good ideas — so this time they got built for real, with the reliability ritual applied from day one.

Pub/sub lives on a dedicated connection (a subscribed Redis connection can't run normal commands — the library manages that for you), and its sacred test kills the subscriber server-side and proves messages flow again after recovery.

Distributed locking is SET NX PX with a random token, and release/extend go through Lua scripts that check the token — a holder whose lock expired can never delete the new holder's lock. The README says, in bold, that this is a single-instance mutex and not Redlock. Honesty is a feature.

And they compose. The cache helper uses the lock internally for stampede protection:

const report = await redis.getOrSetJson('report:daily', 3600, buildReport, { lock: true })
Enter fullscreen mode Exit fullscreen mode

Ten concurrent cache misses, one producer call — the test proves exactly that, with real concurrency against a real Redis.

The reviews that found what I couldn't

"Looking finished" is still the most dangerous state, so finished code got adversarial reviews — every finding had to be reproduced live before I accepted it. Two highlights that survived the process:

A producer returning undefined in the cache helper poisoned the key: JSON.stringify(undefined) is undefined, which got stored as an empty string with a TTL — and every later read exploded with SyntaxError until it expired. One absent-minded return and the key is bricked for an hour.

And the packaging check — install the actual tarball in a clean project — caught two defects no unit test could: my prepare script made npm v12 nag every consumer with an allow-scripts approval warning, and TypeScript consumers in CommonJS hit TS1479 because a "type": "module" package needs dual declaration files (index.d.ts + index.d.cts). Validate the artifact, not the repo.

What shipped

  • 50 tests: 24 unit, 26 integration against a real Redis — including server-side CLIENT KILL recovery for both commands and pub/sub
  • 1 runtime dependency (ioredis). The old version had three: pino and pino-pretty shipped a dev log formatter to every production install. Bring your own logger; the fallback is 25 dependency-free lines
  • Fail-fast structured errors, observable lifecycle events, working optimistic locking via dedicated connections
  • CI on Node 22/24 with a real Redis service container, npm trusted publishing (OIDC) with provenance, zero lifecycle scripts in the package

It's on npm as @pinceladasdaweb/redis, source at github.com/pinceladasdaweb/redis.

The lessons, second edition

  1. Docs are a spec you've already published. Either the code honors them or the docs change. My README described a different, better library — and the gap was invisible because nothing tested the claims.
  2. Read everything before fixing anything. The four worst bugs looked independent and were one defect: a reconnection layer that shouldn't exist. Patch-by-patch would have polished each symptom and kept the disease.
  3. Don't own what your driver owns. The best resilience code I wrote this month was a deletion. If you're wrapping a client that already reconnects, your job is what happens around reconnection — state, events, dedicated connections — not a competing loop.
  4. A write that resolves while doing nothing is data loss with good manners. Fail fast, fail loud, fail with a code.
  5. Kill it from the server side. CLIENT KILL is one command. If your library claims resilience and your test suite has never dropped a connection for real, you don't have a resilience feature — you have a resilience rumor.

Same ending as last time, because it's still true: this library now survives everything I could think of throwing at it, and it has zero production mileage under other people's workloads. If you run Redis with Node and feel like breaking something, I'd genuinely love the bug reports.

Turns out "looking finished" is still the bug. At least this time I knew where to look.

Top comments (0)