Skip to content

refactor: cleanup error checks in Windows#6

Merged
murfffi merged 1 commit into
mainfrom
refactor/windowscond
May 24, 2026
Merged

refactor: cleanup error checks in Windows#6
murfffi merged 1 commit into
mainfrom
refactor/windowscond

Conversation

@murfffi

@murfffi murfffi commented May 24, 2026

Copy link
Copy Markdown
Owner

No description provided.

Also add tests and add copyright section to README

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the conncheck library by adding a copyright section to the README, improving error handling in the core logic, and expanding test coverage. Specifically, it adds a nil check for SyscallConn results and refactors the Windows-specific peek logic for better error reporting. Feedback suggests further refining the Windows implementation by explicitly checking for socket-related errors like WSAENOTSOCK or WSAENOTCONN to return a more accurate status when setting socket options fails.

Comment thread peek_windows.go
Comment on lines 64 to 68
if sockOptErr != nil {
// We couldn't set the timeout and do the check.
// TODO: Determine if the reason is that the socket is closed and return NotOpen in that case
return StatusUnknown
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When sockOptErr is non-nil, the function currently returns StatusUnknown. However, if the error indicates that the socket is closed, not connected, or has been shut down (e.g., WSAENOTSOCK, WSAENOTCONN, or WSAESHUTDOWN), we can definitively return StatusNotOpen. This addresses the TODO on line 66 and improves the accuracy of the liveness check on Windows by reducing false positives (where a closed connection might be assumed open).

if sockOptErr != nil {
	if errors.Is(sockOptErr, windows.WSAENOTSOCK) ||
		errors.Is(sockOptErr, windows.WSAENOTCONN) ||
		errors.Is(sockOptErr, windows.WSAESHUTDOWN) {
		return StatusNotOpen
	}
	return StatusUnknown
}

@murfffi murfffi merged commit 573b660 into main May 24, 2026
6 checks passed
@murfffi murfffi deleted the refactor/windowscond branch May 24, 2026 06:08
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.

1 participant