feat(cli): reconnect instead of dying when the app side goes away - #246
Merged
Conversation
Field report: socket drops are systemic (all connections at once) and reconnecting heals them — which matches the app process restarting (rolling auto-update or crash) plus watch-events' hard exit after one hour of silence (the 3600s idle read timeout was fatal). Three CLI-side changes, no server protocol changes (a server ping would break older CLIs piping the event stream): - watch-events now reconnects indefinitely on EOF, transport error, or the idle timeout: one stderr notice per drop (with elapsed seconds, so field reports distinguish the 1h idle case from an app restart), exponential backoff 0.5s to a 5s cap with jitter, full connect + re-auth + resubscribe on the same path as the initial connect, and a stderr notice on success. stdout stays events-only. --no-reconnect preserves the old exit-on-drop behavior. - one-shot commands retry the initial connect up to 3 times over ~1.5s when it fails with ECONNREFUSED/ENOENT (the app-is-restarting window). Only the connect syscall is retried, never anything after a successful connection, so non-idempotent commands cannot double-execute. - connect failures now carry errno (SocketConnectError) instead of message strings, so the retry logic classifies transient vs permanent without parsing. No automated test: bouncing the app's own socket listener mid-test has no harness in tests_v2 (existing reconnect tests bounce docker sshd or the browser proxy, never the app process), and the repo policy keeps the app under test a single tagged instance. Stated per test policy instead of faking it.
…etrying permanent failures watch-events's subscribeAndAck() now parses the subscribe ack and requires ok:true before treating a (re)connect as subscribed -- a rejected resubscription (bad password, unsafe-socket rejection, invalid params) no longer prints 'reconnected' and the raw ack line is never echoed to stdout (stdout is events-only). The reconnect retry loop now writes a stderr line for every failed attempt instead of failing silently, and classifies auth/protocol rejections as permanent: after 3 consecutive permanent failures it exits with the underlying error instead of looping forever. Transient failures (ECONNREFUSED/ENOENT/EOF/timeout) keep retrying indefinitely as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Hardens the CLI against the 'sockets are unstable, losing connection constantly' report. The drop pattern (systemic, all at once, reconnect heals) matches the app process restarting — we ship a rolling build on every green commit, so update relaunches are frequent — plus watch-events hard-exiting after one hour of event silence.
Deliberately no server protocol changes: a server-side keepalive/ping event would leak into older CLIs' piped event streams (version skew during rolling updates).
Test plan
No automated test: bouncing the app's own listener mid-test has no tests_v2 harness (existing reconnect tests bounce docker sshd / the browser proxy, never the app), stated per test policy rather than faking one.