fix(#707): route the remaining test servers through a loopback listen helper - #708
Open
Weegy wants to merge 3 commits into
Open
fix(#707): route the remaining test servers through a loopback listen helper#708Weegy wants to merge 3 commits into
Weegy wants to merge 3 commits into
Conversation
… helper #703 fixed the mechanism but could only convert the call sites that already waited for their `listening` callback. The remaining 57 read `server.address().port` synchronously on the next line, which stops working the moment a host is passed: `listen` then goes through the `dns.lookup` path even for an IP literal and no longer binds synchronously. They now go through `test/_helpers/listenLoopback.ts`, which binds 127.0.0.1 and resolves on `listening`. Enclosing hooks and three helper functions became async to match. canvas-core's WebSocket stub server had the same shape and now binds the loopback too. Also removed 23 now-dead `await once('listening')` waits that followed a converted site: the helper already resolves after `listening`, so the second wait could never fire and hung 12 files to the 120s test timeout. Those files had been awaiting correctly all along -- they only ever needed the host. Corrects #703's explanation. That entry claimed the wildcard socket is IPV6_V6ONLY. Measured on macOS it is not: `[::]` is dual-stack and http://127.0.0.1:<port> normally reaches it, which is why the bug presented as intermittent rather than as a hard failure. The real mechanism is that the wildcard bind's port is chosen only against other wildcard binds, while a process binding 127.0.0.1:<port> specifically may already hold it -- and on BSD/macOS the more specific bind coexists with the wildcard and wins for connections to 127.0.0.1. Local dev servers bind 127.0.0.1 by default, which is why the observed shadowers were an MCP server and a Flask app. Closes #707
…-sweep # Conflicts: # docs/CHANGELOG.md
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.
Closes #707. Follow-up to #703.
What this finishes
#703 fixed the mechanism behind the intermittent
401/404/HTTPParserErrorin the middleware suite, but could only convert the call sites that already waited for theirlisteningcallback. The other 57 readserver.address().portsynchronously on the next line, which stops working the moment a host is passed —listenthen goes through thedns.lookuppath even for an IP literal and no longer binds synchronously.All of them now go through
test/_helpers/listenLoopback.ts, which binds127.0.0.1and resolves onlistening. No test server undermiddleware/test/is left holding a port it never dials.A correction to #703
#703 said the wildcard socket is
IPV6_V6ONLY. Measured on macOS, it is not.[::]is dual-stack andhttp://127.0.0.1:<port>normally reaches it — which is precisely why the bug presented as intermittent rather than as a hard failure. Had the socket really been v6-only, every one of these tests would have failed every time.The real mechanism:
The wildcard bind's port is chosen only against other wildcard binds. A process that binds
127.0.0.1:<port>specifically may already hold that exact port, and on BSD/macOS the more specific bind coexists and wins for connections addressed to 127.0.0.1. Local dev servers bind 127.0.0.1 by default — which is exactly why the shadowers observed while diagnosing #703 were an MCP server (401 … provide valid authorization token) and a Flask app (404 <!doctype html>).The fix and its rationale are unchanged; only the description of why the port was unprotected was wrong. Corrected in the changelog, in the helper's doc comment, and in both dev-platform harnesses, which now delegate to the helper rather than carrying their own copy of the explanation.
A bug this surfaced
12 files hung to the 120s test timeout after the first sweep. They contained:
The helper already resolves after
listening, so the second wait could never fire. 23 such waits removed. Worth noting: those files had been awaiting correctly all along — they only ever needed the host, not the await.Also
canvas-core's WebSocket stub server had the same shape (new WebSocketServer({ port: 0 })with callers diallingws://127.0.0.1:<port>) and now binds the loopback. Its suite is 172/172.middleware/test/guardedOutboundFetch.test.tsis left alone deliberately: it binds127.0.0.1and dialslocalhoston purpose, exercising the resolver path for the DNS-rebinding guard, and only asserts rejection.Verification
npm run testmain(incl. #702, #704)npm run test:updaternpm run test -w @omadia/canvas-corenpm run typecheck:testnpm run lintcore decoupling ratchet (#470)mergeable_state: cleanMutation check: pointing the helper at
'::'instead of'127.0.0.1'turns all 3 binding guards red.No retries, sleeps, timeout bumps, or skips. Lockfile untouched.