Skip to content

fix(whois): check conn.Close return in tcp43RawDial (CI red on main) - #141

Merged
noah-tutt-praetorian merged 1 commit into
mainfrom
anushkavirgaonkar/fix-errcheck-tcp43-conn-close
Aug 7, 2026
Merged

fix(whois): check conn.Close return in tcp43RawDial (CI red on main)#141
noah-tutt-praetorian merged 1 commit into
mainfrom
anushkavirgaonkar/fix-errcheck-tcp43-conn-close

Conversation

@anushkavirgaonkar

Copy link
Copy Markdown
Contributor

Summary

CI has been failing on main since the Consolidate WHOIS commit (4f7fead). The Lint job fails with a single errcheck issue; every other job in the run is green.

pkg/whois/tcp43.go:139:18: Error return value of `conn.Close` is not checked (errcheck)
	defer conn.Close()
	                ^
1 issues:
* errcheck: 1

Failing run: https://github.com/praetorian-inc/pius/actions/runs/31187027446

One line, in tcp43RawDial:

-	defer conn.Close()
+	defer func() { _ = conn.Close() }()

Why this only shows up in CI

CI pins golangci-lint v2.12.2. v2 dropped the legacy default exclusions that v1 shipped, one of which (EXC0001) excluded .*Close. Anyone still on a v1 linter locally will see this line pass, which is likely how it reached main. Re-running v1 with exclude-use-default=false reproduces the CI error character-for-character, including the 139:18 position, and reports clean after the fix.

conn.Close() on a read-only WHOIS socket has no error worth acting on, so the return is discarded explicitly rather than logged. This matches the idiom already in the repo (pkg/cache/cache.go:89,100,108, pkg/plugins/cidrs/rpsl.go:107) and the _ = conn.SetDeadline(deadline) line six lines below it in the same function.

JIRA or Linear

None. This is an unblock for red main, not tracked work. Happy to file one if the team wants a record.

Verification

  • go build ./... clean
  • go vet ./pkg/whois/... clean
  • go test ./pkg/whois/... -> ok github.com/praetorian-inc/pius/pkg/whois
  • errcheck red/green reproduced locally as described above

No test added: the deferred call runs identically before and after, only the discarded return is now explicit, so no test could distinguish the two. CI's own Lint job is the real check.

🤖 Generated with Claude Code

golangci-lint v2.12.2 flags the bare `defer conn.Close()` under errcheck,
turning CI red on main since the "Consolidate WHOIS" commit (4f7fead).

v2 dropped the legacy default exclusions that v1 shipped, including
EXC0001 (`.*Close`), so this line lints clean locally on v1 and fails in
CI. Discard the return explicitly, matching the idiom already used in
pkg/cache/cache.go and pkg/plugins/cidrs/rpsl.go, and the
`_ = conn.SetDeadline(...)` line in this same function.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 45d94954-ba53-47cf-81f8-cb2e6f13ca57

📥 Commits

Reviewing files that changed from the base of the PR and between 4f7fead and e84971f.

📒 Files selected for processing (1)
  • pkg/whois/tcp43.go

Walkthrough

The TCP WHOIS connection cleanup now uses a deferred function to call conn.Close() and discard its error. No exported declarations changed.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch anushkavirgaonkar/fix-errcheck-tcp43-conn-close

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Review

Critical issues

None.

Security

No security concerns flagged.

Test coverage

No test needed — the deferred call behaves identically before and after; only the discarded return is now explicit. Matches the existing _ = conn.SetDeadline(...) idiom in the same function (tcp43.go:145), and CI Lint (golangci-lint v2.12.2, which drops the legacy EXC0001 .*Close exclusion) is the real check.

No critical issues — LGTM pending human review.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gemini Review

No critical issues — LGTM pending human review.


Reviewed by Gemini (gemini-3.1-pro-preview)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex Review

No critical issues — LGTM pending human review.


Reviewed by Codex (gpt-5.5)

@noah-tutt-praetorian
noah-tutt-praetorian merged commit 823dcd2 into main Aug 7, 2026
25 checks passed
@noah-tutt-praetorian
noah-tutt-praetorian deleted the anushkavirgaonkar/fix-errcheck-tcp43-conn-close branch August 7, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants