Skip to content

valkey: close the socket on every fail() and mark the client disconnected before onclose runs - #37993

Closed
alii wants to merge 27 commits into
mainfrom
ali/valkey-fail-recovery
Closed

valkey: close the socket on every fail() and mark the client disconnected before onclose runs#37993
alii wants to merge 27 commits into
mainfrom
ali/valkey-fail-recovery

Merge remote-tracking branch 'origin/main' into ali/valkey-fail-recovery

1909276
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 15, 2026 in 34m 32s

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/js_valkey.rs:1145-1152 tls_ctx_failed branch in connect() calls on_valkey_close() inline — same-class recursion sibling left unfixed

Annotations

Check warning on line 1152 in src/runtime/valkey_jsc/js_valkey.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

tls_ctx_failed branch in connect() calls on_valkey_close() inline — same-class recursion sibling left unfixed

`connect()`'s `tls_ctx_failed` branch (js_valkey.rs ~1509-1519, unchanged) still calls `on_valkey_close()` inline — the exact class `close_without_socket_next_tick()` was introduced for. An `onclose` that unconditionally re-dials against a `tls: {…custom…}` config whose SSL_CTX build fails deterministically (e.g. unreadable `ca`/`cert` path) recurses until the stack runs out, since `do_connect()` sees no cached promise (just cleared), status `Disconnected` (the `Connecting` write is after this b