redis: re-issue SUBSCRIBE after an auto-reconnect - #33450
Open
robobun wants to merge 11 commits into
Open
Claude / Claude Code Review
completed
Jul 6, 2026 in 32m 47s
Code review found 1 potential issue
Found 2 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/runtime/valkey_jsc/valkey.rs:1113-1121 |
Error reply to promise-less resubscribe steals an unrelated command's promise slot |
Annotations
Check warning on line 1121 in src/runtime/valkey_jsc/valkey.rs
claude / Claude Code Review
Error reply to promise-less resubscribe steals an unrelated command's promise slot
`resubscribe_pending` only accounts for the success path (a `Push` with kind `subscribe`). If the server replies to the replayed `SUBSCRIBE` with an error — e.g. `-NOPERM` after an ACL change during the outage — the reply is a `RESPValue::Error`, so this block is skipped, `should_consume_promise_pair` stays true, and `in_flight.read_item()` pops the pair belonging to whatever user command landed in `in_flight` (e.g. a `ping()` from an `onconnect` handler). The subscriber `Error` arm then calls `
Loading