fix(netstat): handle missing /proc/net/tcp6 on IPv6-disabled hosts#706
Conversation
On Linux hosts booted with ipv6.disable=1 or built without CONFIG_IPV6, /proc/net/tcp6 and /proc/net/udp6 do not exist. doNetstat previously returned the os.IsNotExist error verbatim, which propagated through TCP6Socks and caused findPorts to discard already-collected IPv4 sockets and fail the entire watch loop — leaving IDEs such as openvscode unreachable because no ports were ever forwarded. Treat a missing procfile as "zero sockets of this family" by returning (nil, nil) only when os.IsNotExist is true. Permission errors, I/O errors, and parse errors continue to propagate unchanged. Adds the first unit tests for pkg/netstat, covering the missing-file path, the happy path, non-IsNotExist open errors, and parse errors. Fixes skevetter#705 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughdoNetstat now treats missing target files as non-fatal by returning (nil, nil) when os.Open reports os.IsNotExist; other open errors and parse errors are still returned. A new test suite exercises missing-file, valid-parse, permission-open, and parse-error cases. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes the netstat-based port watcher on Linux hosts where IPv6 is disabled (or not built into the kernel) by treating missing /proc/net/tcp6/udp6 procfiles as “no sockets” instead of a hard error, preventing IPv4 results from being discarded.
Changes:
- Update
doNetstatto return(nil, nil)whenos.Openfails withos.IsNotExist, enabling graceful degradation when IPv6 procfiles are absent. - Add a new test suite for
doNetstatcovering missing-file behavior, successful parsing, non-IsNotExistopen errors, and parse errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/netstat/netstat_util.go | Gracefully handles missing proc netstat files by treating IsNotExist as empty results. |
| pkg/netstat/netstat_util_test.go | Introduces regression tests ensuring the new behavior is correct and surgical. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/netstat/netstat_util.go (1)
251-253: Align “empty slice” wording with actual return value.Line 251–253 says “empty slice,” but Line 259 returns
nil. Either return[]SockTabEntry{}or reword to “zero entries” to avoid ambiguity.Also applies to: 259-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/netstat/netstat_util.go` around lines 251 - 253, The comment claims the function returns an “empty slice” on systems without IPv6 but the code returns nil; change the return at the nil site (currently returning nil for []SockTabEntry) to an actual empty slice ([]SockTabEntry{}) and update the comment around that block to match, referencing the SockTabEntry slice return so callers get a real empty slice instead of nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/netstat/netstat_util.go`:
- Around line 251-253: The comment claims the function returns an “empty slice”
on systems without IPv6 but the code returns nil; change the return at the nil
site (currently returning nil for []SockTabEntry) to an actual empty slice
([]SockTabEntry{}) and update the comment around that block to match,
referencing the SockTabEntry slice return so callers get a real empty slice
instead of nil.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8836361-472e-402e-b861-94072dfccede
📒 Files selected for processing (2)
pkg/netstat/netstat_util.gopkg/netstat/netstat_util_test.go
|
Whenever you're ready for more testing, the Desktop app and CLI artifacts are created each run (example https://github.com/skevetter/devpod/actions/runs/24168163306?pr=706) |
|
@skevetter Just tested, works fine now! |
Address CodeRabbit review: comments said "empty slice" but the code returns nil. Reword to say "(nil, nil)" for accuracy.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/netstat/netstat_util_test.go (1)
23-34: Tighten the missing-file assertion to pin the exact contract.Line 33 currently allows both
niland[]viaassert.Empty, but the implementation contract here is specifically(nil, nil).♻️ Suggested tweak
- assert.Empty(s.T(), entries, "missing procfile must yield zero entries") + assert.Nil(s.T(), entries, "missing procfile should return nil entries")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/netstat/netstat_util_test.go` around lines 23 - 34, The test TestDoNetstat_MissingFileReturnsEmpty should assert the exact contract that doNetstat returns a nil slice (not just empty) when the proc file is missing: replace the permissive assert.Empty on the variable entries with an assertion that entries is nil (e.g. assert.Nil or require.Nil) while keeping the existing assert.NoError on err so the test verifies (nil, nil) precisely for doNetstat.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/netstat/netstat_util_test.go`:
- Around line 23-34: The test TestDoNetstat_MissingFileReturnsEmpty should
assert the exact contract that doNetstat returns a nil slice (not just empty)
when the proc file is missing: replace the permissive assert.Empty on the
variable entries with an assertion that entries is nil (e.g. assert.Nil or
require.Nil) while keeping the existing assert.NoError on err so the test
verifies (nil, nil) precisely for doNetstat.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fb48594-f913-4cbe-9035-a3d6012347c5
📒 Files selected for processing (2)
pkg/netstat/netstat_util.gopkg/netstat/netstat_util_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/netstat/netstat_util.go
Summary
Fixes #705. On Linux hosts where IPv6 is disabled at boot (
ipv6.disable=1) or absent from the kernel (CONFIG_IPV6=n),/proc/net/tcp6does not exist. The devpod agent's netstat-based port watcher was returning theos.IsNotExisterror verbatim fromdoNetstat, which propagated throughTCP6Socksand causedwatcher.findPortsto discard the already-collected IPv4 sockets and return an error on every tick. The net effect was that no ports were ever forwarded and IDEs like openvscode were unreachable.Fix
pkg/netstat/netstat_util.go— indoNetstat, treatos.IsNotExistfromos.Openas "zero sockets of this family" and return(nil, nil). Matches the repo's existing graceful-degradation pattern (seepkg/gitsshsigning/helper.go,pkg/workspace/provider.go,pkg/envfile/envfile.go). All other errors — permission, I/O, parse — still propagate unchanged.Applied once in the common helper rather than per-caller so that
osTCPSocks,osTCP6Socks,osUDPSocks, andosUDP6Socksall benefit uniformly.Tests
Adds
pkg/netstat/netstat_util_test.go— the first test file in this package — with four TDD-driven tests:TestDoNetstat_MissingFileReturnsEmpty— red-phase driver; fails before the fix, passes after.TestDoNetstat_ParsesValidFile— pins the happy path with a synthetic/proc/net/tcp6-format entry ([::]:22LISTEN) so the fix cannot accidentally suppress successful parses.TestDoNetstat_PropagatesNonNotExistOpenError— proves the fix is surgical by confirming permission-denied errors still bubble up. Skipped on Windows and when running as root.TestDoNetstat_PropagatesParseError— confirms malformed content still errors.Tests use the
testify/suitepattern consistent withpkg/gitsshsigning/helper_test.goand rely only ont.TempDir(), so they are OS-agnostic.Test plan
go test ./pkg/netstat/... -run TestNetstatUtilSuite -vpasses (all 4 tests)task cli:testequivalent (go test ./... -race -coverprofile=dist/profile.out -covermode=atomic, excluding e2e) passestask cli:lint:new(golangci-lint--new-from-rev origin/main) reports 0 new issuesgo vet ./pkg/netstat/...cleanipv6.disable=1: start a workspace, confirm openvscode is reachable and the agent log no longer shows repeatedopen /proc/net/tcp6: no such file or directoryentries from the watcher.Follow-ups (not in this PR)
watcher.findPortsstill discards IPv4 results ifTCP6Sockserrors for any non-IsNotExist reason. Worth hardening, but requires injecting a socket source intoWatcher— out of scope for a bug fix. Happy to open a separate issue.Fixes #705
Summary by CodeRabbit
Bug Fixes
Tests