Skip to content

fix: never let a Redis error in the exit path crash the process - #26

Closed
beagleknight wants to merge 1 commit into
mainfrom
fix/resilient-redis-close
Closed

fix: never let a Redis error in the exit path crash the process#26
beagleknight wants to merge 1 commit into
mainfrom
fix/resilient-redis-close

Conversation

@beagleknight

Copy link
Copy Markdown
Collaborator

Context

During a short Redis brownout on the ci cluster (2026-07-23 09:29-09:34Z, factorial run 29993494537), five green E2E shards turned red: every test passed (Failing: 0), [specbandit:cypress] Teardown complete. printed, and ~5 seconds later the process died with:

/code/node_modules/.pnpm/ioredis@5.10.1/node_modules/ioredis/built/Command.js:194
                    this.reject(new Error("Command timed out"));
Error: Command timed out
    at Timeout.<anonymous> (.../ioredis/built/Command.js:194:33)

The failure chain

  1. The worker's final "any more work?" steal hit the brownout and exhausted its 5 retries → run() threw (correct: the worker can't confirm the queue is drained).
  2. The CLI's finally called queue.close()redis.quit(). QUIT is a regular command, so it hit the same brownout and its commandTimeout rejected 5s later — replacing the original error.
  3. bin.ts had no top-level catch, so the rejection escaped the top-level await and Node 22 killed the process with a raw stack, masking the real cause.

Changes

  • RedisQueue#close() never throws: QUIT failures fall back to a hard disconnect() (drops the socket and pending commands without a round-trip). close() runs in exit paths — a throw there can only mask the real error or crash an otherwise-green run.
  • bin.ts top-level try/catch: clean [specbandit] Fatal: <message> instead of a raw ioredis stack, deterministic exit 1.
  • withRetries wraps the final error with operation + attempt count (Redis steal failed after 5 attempts: Command timed out) so CI logs are attributable at a glance.

Testing

  • New test: close() resolves and calls disconnect() when QUIT rejects.
  • Updated the exhausted-retries test for the wrapped message.
  • vitest run: 149 passed; the 2 failures (cypressAdapter/jestAdapter "not installed" tests) also fail on clean main locally — pre-existing, unrelated.

A CI incident (2026-07-23, factorial run 29993494537) turned five green
E2E shards red: every test passed, teardown completed, and then the
process died with an uncaught 'Command timed out' from ioredis.

The chain: the final steal exhausted its retries during a short Redis
brownout and threw; the CLI's finally block then called queue.close(),
whose QUIT command hit the same brownout and timed out 5s later. That
rejection replaced the original error and escaped through the top-level
await in bin.ts, which had no catch — so Node printed a raw stack and
the real failure was masked.

Three changes:
- RedisQueue#close() never throws: QUIT failures fall back to a hard
  disconnect(), dropping the socket and pending commands.
- bin.ts wraps CLI.run in a top-level try/catch printing a clean
  '[specbandit] Fatal:' message with a deterministic exit 1.
- withRetries wraps the final error with the operation name and attempt
  count, so exhausted retries are attributable in CI logs.
Samielakkad

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants