Skip unroutable IP families before dialing#134
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the proxy’s TCP dialing behavior on single-stack hosts by filtering out DNS-resolved IPs whose address family the host can’t route, avoiding long socket timeouts and reducing cascading request timeouts in upstream clients.
Changes:
- Added IPv4/IPv6 “route availability” probes (cached for process lifetime) and filtered resolved IPs accordingly before dialing.
- Introduced
ErrNoUsableAddressto fail fast when DNS returns only unusable addresses for the requested network. - Added unit tests covering single-stack behavior, requested network filtering (
tcp4/tcp6), and “neither family available” cases.
Show a summary per file
| File | Description |
|---|---|
| internal/dialer/dialer.go | Filters resolved IPs by routable family before dialing; adds availability probes and a new error for “no usable address”. |
| internal/dialer/dialer_test.go | Adds tests validating address-family filtering and error behavior without attempting any real dials. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
Comment on lines
+68
to
72
| ips, err = d.usableIPs(network, host, ips) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| for _, ip := range ips { |
Comment on lines
+14
to
+22
| func TestDialContextSkipsIPv6WhenUnavailable(t *testing.T) { | ||
| dialer, dialed := testDialer([]string{"2001:db8::1", "192.0.2.1"}, false) | ||
|
|
||
| conn, err := dialer.DialContext(context.Background(), "tcp", "example.com:443") | ||
| require.NoError(t, err) | ||
| require.NotNil(t, conn) | ||
|
|
||
| assert.Equal(t, []string{"192.0.2.1:443"}, *dialed) | ||
| } |
JamieMagee
force-pushed
the
fix/symmetric-ip-family-filtering
branch
from
925e48f to
74734c6
Compare
h5dh2gy26z-sys
approved these changes
Jun 4, 2026
brettfo
reviewed
Jun 4, 2026
brettfo
approved these changes
Jun 4, 2026
|
I am guessing that this superseded and made #124 redundant, right? |
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.
What are you trying to accomplish?
On IPv4-only hosts the proxy still tried IPv6 addresses returned by DNS, hung until the socket timed out, and that cascaded into the updater's 100s
HttpClient.Timeout. Now we drop addresses whose family the host can't route before dialing, so single-stack hosts fail fast. IPv6-only hosts had the same problem with IPv4 addresses, so both families get the same treatment.Anything you want to highlight for special attention from reviewers?
The probe is a connected UDP dial to a documentation address (
192.0.2.1/2001:db8::1). It checks for a route, not real reachability, and caches the result for the process lifetime. A blackhole route could still report "available" (same caveat for both families).How will you know you've accomplished your goal?
go test ./internal/dialer/passes, including new cases: IPv4-only keeps only IPv6, IPv6-only keeps only IPv4, and neither available returnsErrNoUsableAddresswithout dialing.Checklist