test(bridge): move port-discovery fixture ports out of the OS ephemeral port range - #197
Merged
Merged
Conversation
…al port range port-discovery.unit.test.ts's hardcoded fixture ports (49000-49402) sat inside the ephemeral/dynamic port range both Linux (32768-60999) and macOS (49152-65535) draw from when auto-assigning a source port to an outbound connection. An unrelated process's outbound socket could transiently claim one of those exact port numbers at the moment a test tried to bind it as a listening server, producing a genuine EADDRINUSE race that a prior retry-window widening didn't fully close out. Move every fixture port down to the 20000s, below both platforms' ephemeral floors, removing the collision domain outright. Add a test asserting all fixture ports stay under that ceiling, and a deterministic test proving blockPort's own EADDRINUSE retry recovers from a genuinely transient bind conflict rather than relying on it happening not to occur. Closes #196
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🎉 This PR is included in version 3.17.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Fixes the EADDRINUSE flake reported in #196:
port-discovery.unit.test.ts's hardcodedfixture ports (49000-49402) sat inside the ephemeral/dynamic port range both Linux
(default 32768-60999) and macOS (default 49152-65535) draw from when auto-assigning a
source port to an outbound connection. Any unrelated process's outbound socket could
transiently claim one of those exact port numbers at the moment a test tried to bind it
as a listening server. A prior fix (widening blockPort's own EADDRINUSE retry window)
reduced the odds but didn't close out the race, since it only covers the test's own
bind attempts, not the underlying reason the collision domain existed in the first
place.
This moves every fixture port down to the 20000s, below both platforms' ephemeral
floors, removing the collision domain outright rather than tolerating it with a wider
retry window. Adds a test asserting every fixture port stays under that ceiling (fails
against the old 49000s range, passes against the new one), and a deterministic test
proving
blockPort's own retry genuinely recovers from a transient EADDRINUSE ratherthan relying on that path never being exercised.
Closes #196