Repository navigation
internal/vm: a failed connect keeps its errno, and a name that does not resolve says so - #806
Merged
Merged
Conversation
…ot resolve says so
rbgo raised SocketError for everything that went wrong on the way to a peer,
with "getaddrinfo: " in front of a Go error -- a label that is simply false for
a refused connection, where no name was being resolved.
The cost is that the ordinary idioms stop working in BOTH directions: code that
rescues Errno::ECONNREFUSED to back off and retry never matches, and code that
rescues SocketError to report a bad hostname matches a perfectly resolvable
host whose port is shut.
MRI 4.0.7 has three answers where rbgo had one, measured here:
TCPSocket.new("127.0.0.1", <closed>) Errno::ECONNREFUSED
TCPSocket.new("no-such-host.invalid", 80) Socket::ResolutionError
Socket.tcp("192.0.2.1", 80, timeout:) Errno::ETIMEDOUT
The first two now match, class AND message:
Connection refused - connect(2) for "127.0.0.1" port 1
is byte for byte what MRI prints. (The third needs Socket.tcp, which rbgo does
not have; out of scope here.)
Socket::ResolutionError is new, and it is a SUBCLASS of SocketError -- so every
`rescue SocketError` written before this change still catches a resolution
failure. A test asserts that rather than leaving it to be noticed.
Both raisers had to change, which is why the classifier is shared:
TCPSocket.new goes through socket.go and Net::HTTP -- the path #772 was filed
on -- through raiseTransportErr in nethttp_bind.go. The Net::HTTP side passes no
address, because its phase is not always a connect and naming connect(2) there
would describe a call that did not happen.
The tests get their closed port by binding a listener and closing it. "Port 1 is
surely shut" is a statement about the host a test happens to run on.
Closes #772.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tests stop asserting the old class Two changes the dial work uncovered, kept here because they are the same measurement session. #774 (SystemExit half). `fork { exit 7 }` ended the WHOLE program at the fork call: the line after the fork never ran and the process exited 7. MRI runs on and reports 7 through Process.wait2. rbgo runs the block in-process BY DESIGN, so the block IS the child -- and the child's exit is runForkBlock returning the status, not a panic escaping to the top. exit, exit! and abort all travel the same path and all three now match, with a block that returns normally still 0 as the control. This does NOT close #774: an UNCAUGHT exception in the block still takes the parent down, where MRI prints it in the child and exits 1. That needs an exception renderer inside the VM, which lives in the CLI today, and is left deliberately rather than half-done. Three more tests asserted the defect this branch fixes, all rescuing SocketError for a refused connection: nethttp_bind_test.go TestNetHTTPErrors nethttp_bind_test.go TestNetHTTPDialXferError (three arms) socket_test.go TestSocketErrors Each was green exactly while every way of failing to reach a peer arrived as one class, so fixing the tool turned them red. They now assert MRI's class rather than being loosened -- and TestNetHTTPErrors asserts BOTH classes separately, since Errno::ECONNREFUSED alone would not notice a name failure being given the same one. Found by running the whole suite and COUNTING the failures, after a filtered grep had reported the first of them as green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ing already was The coverage gate refused the branch at 91.7%, naming the SystemCallError arm -- an errno with no Errno::Exxx class registered for it. Unreached, not dead: WHICH errnos have a class depends on what the platform's syscall package defines, so one outside that set is reachable in principle and must not quietly become something else. raiseChdirErr has the same arm and is at 100%, covered by an internal test that hands it an errno no platform table claims (dir_getwd_internal_test.go). This uses that idiom rather than inventing a second one, and adds three cases the dial path has and chdir does not: the address still reaches the message (an arm that dropped it would leave a caller unable to say WHICH peer failed), the no-address form invents no connect(2) suffix, and a DNSError is a resolution failure whatever errno it may also wrap -- which is why that check comes first. indexOf was already defined in this package, in wave28_flock_internal_test.go. The second copy is not in this commit; strings.Contains is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The windows lane refused the previous commit, and it was right. Windows fails
socket calls with WSAE* codes in the 10000 range, which errnoClasses does not
name, so a refused connection arrived there as a bare SystemCallError:
got "SystemCallError"
want "Errno::ECONNREFUSED"
darwin and linux were green. `rescue Errno::ECONNREFUSED` -- the retry idiom
this whole change is about -- matched nothing on the one platform nobody could
test by hand, which is the shape of a divergence only one lane can see.
MRI translates the same way (rb_w32_map_errno), which is why a Ruby program
written against POSIX errno classes works unchanged on Windows.
Two things the compiler settled that reading would not have:
The target is syscall's CONSTANT, not the number 61. On Windows Go defines the
POSIX errnos above APPLICATION_ERROR rather than at their Unix values, and
errnoClasses is built from those same constants -- so mapping to the constant
lands in the table and errnoStrerror reports Go's own text for it. Hardcoding 61
would have missed on both counts.
The WSA names come from golang.org/x/sys/windows, because syscall defines only
four of them. The first version used syscall for all fifteen and did not
compile; GOOS=windows go build found that, not CI. x/sys is already a direct
dependency of this package (filestat_unix.go, spawn_native.go).
SCOPE OF THE CLAIM. There is no Windows host here and the local MRI source
extract has no win32/win32.c -- and no version stamp, so it would not settle it
either. So the message assertion now runs only where it was measured, and what
every platform asserts instead is the class, plus the SHAPE of the message:
whatever the platform calls a refused connection, the peer it failed against
must be in the text. Scoping a claim to where it was measured is not the same as
loosening it -- the measured claim stays exact, and the windows lane is the
measurement for the class.
All five targets cross-build: darwin, linux, windows, wasip1, js.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`go tool cover -func=cover.out > cover.txt` is a step of the CI gate I run locally before pushing, and `git add -A` swept its output in. It is a build artefact: nothing reads it, it goes stale the moment anything changes, and a reviewer has no way to tell it apart from a file somebody meant to add. Removing the file is the fix, not a .gitignore entry: ignoring is a safety net, and the net is only reached because `git add -A` was used in the first place. The control that works is reading `git status --short` before committing, which is what caught it here -- one line later than it should have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Oct 10, 2026
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
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.
Closes #772. Takes the SystemExit half of #774.
What it was
rbgo raised
SocketErrorfor everything that went wrong on the way to a peer,with
getaddrinfo:in front of a Go error — a label that is simply falsefor a refused connection, where no name was being resolved.
The cost is that the ordinary idioms stop working in both directions: code
rescuing
Errno::ECONNREFUSEDto back off and retry never matches, and coderescuing
SocketErrorto report a bad hostname matches a perfectly resolvablehost whose port is shut.
Measured
MRI 4.0.7 has three answers where rbgo had one:
TCPSocket.new("127.0.0.1", <closed>)Errno::ECONNREFUSEDErrno::ECONNREFUSEDTCPSocket.new("no-such-host.invalid", 80)Socket::ResolutionErrorSocket::ResolutionErrorSocket.tcp("192.0.2.1", 80, connect_timeout:)Errno::ETIMEDOUTSocket.tcp, absent)And the message is MRI's, byte for byte:
Socket::ResolutionErroris new and is a subclass ofSocketError, soevery
rescue SocketErrorwritten before this still catches a resolutionfailure. A test asserts that rather than leaving it to be discovered.
Both raisers had to change, which is why the classifier is shared:
TCPSocket.newgoes through
socket.go, andNet::HTTP— the path #772 was filed on — throughraiseTransportErr. TheNet::HTTPside passes no address, because its phase isnot always a connect and naming
connect(2)there would describe a call thatdid not happen.
Three tests asserted the defect
All rescued
SocketErrorfor a refused connection — green exactly while everyway of failing to reach a peer arrived as one class, so fixing the tool turned
them red. They now assert MRI's class rather than being loosened, and
TestNetHTTPErrorsasserts both classes separately:Errno::ECONNREFUSEDalone would not notice a name failure being given the same one.
The first of them was reported as green by a filtered
grepover the suiteoutput. They were found by running the whole suite and counting the
failures.
Also here:
fork { exit n }stops the child, not the program (#774)rbgo runs the block in-process by design, so the block is the child, and
the child's exit is
runForkBlockreturning the status rather than a panicescaping to the top.
exit,exit!andabortall travel that path and allthree now match MRI; a block that returns normally is still 0, as the control.
This does not close #774. An uncaught exception in the block still takes
the parent down, where MRI prints it in the child and exits 1. That needs an
exception renderer inside the VM — the one that exists lives in the CLI — and
is left deliberately rather than done halfway.
The tests get their closed port by binding a listener and closing it. "Port 1
is surely shut" is a statement about the host a test happens to run on.
🤖 Generated with Claude Code