cli: classify broker control-channel closure vs dial refusal - #194
Merged
Merged
Conversation
The broker dialer collapsed two distinct handshake failures into one opaque message. A dead control channel (runner gone, or channel reclaimed after the spawning command finished) surfaced as "broker handshake send: broken pipe" or the misleading "broker refused connection (code [])" on recv EOF, while a live broker declining the dial (0xFF byte) used the same "refused" wording. - Add exported sentinel ErrBrokerClosed; Sendmsg EPIPE/ECONNREFUSED and Recvmsg EOF (n==0) now wrap it, so errors.Is keeps working through the http.Transport/url.Error wrapping. - The 0xFF refusal gets its own clear message and no longer prints a bogus "code"; other unexpected response bytes get an explicit hex byte instead. - Add dialer-level tests for EOF, EPIPE, and refusal classification.
- Demote ErrBrokerClosed to unexported errBrokerClosed: the SDK flattens dial errors into text, so error identity never survives end-to-end and an exported sentinel has no consumer. Classification rides the user-visible message; only in-module tests use errors.Is. - Drop the "flashduty: " prefix from the new messages (the SDK adds it), and give the 0xFF refusal its final actionable wording. - Fix "broker parse rights: %w" rendering %!w(<nil>) when the rights parse succeeds but yields no fd. - Reword comments to state production errno behavior (Linux SEQPACKET send -> EPIPE; datagram-style peer death -> ECONNREFUSED) and note the sentinel covers the handshake phase only.
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.
Why
The broker-mode dialer collapsed two distinct handshake failures into one opaque message:
broker handshake send: broken pipeor the misleadingbroker refused connection (code [])when the recv returned EOF;What
ErrBrokerClosed.SendmsgEPIPE/ECONNREFUSED andRecvmsgEOF (n==0) now wrap it, so callers and automation can classify "broker gone" programmatically viaerrors.Is— which keeps working through thehttp.Transport/url.Errorwrapping, and the user-visible message now explains that the broker that started the process is no longer available.broker refused the dial request) and is deliberately notErrBrokerClosed; the boguscode []formatting is gone, and any other unexpected response byte is reported as an explicit hex byte.Testing
go test -race ./...(darwin)GOOS=linux go build ./...+GOOS=linux go vet ./internal/cli/(compiles the unix-tagged test files for linux too)make(gofmt + gci + golangci-lint 0 issues + race tests + build)Known limitation (pre-existing, out of scope)
The handshake has no deadline: if the broker is alive but never answers a handshake request, the dial blocks until the caller's context/HTTP timeout fires. Unchanged by this PR.