fix: don't log warn/errors on connection close after a legit close #123

Merged
nemith merged 1 commit from brb/push-owvyzmtwlyxz into main 2026-01-07 17:51:49 +00:00
nemith commented 2026-01-07 17:22:14 +00:00 (Migrated from github.com)

I have been putting this off but this will silence errors when the user closes the connection and will only log on unexpected transport closes.

This also moved closing to a atomic.Bool to be a bit cleaner and have less lock contention. Probably doesn't matter.

I have been putting this off but this will silence errors when the user closes the connection and will only log on unexpected transport closes. This also moved closing to a atomic.Bool to be a bit cleaner and have less lock contention. Probably doesn't matter.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-01-07 17:27:15 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR improves error logging during connection closure by suppressing warnings and errors when a session is closed gracefully. The implementation changes the closing field from a mutex-protected boolean to an atomic.Bool for cleaner concurrency control and reduced lock contention.

Key Changes:

  • Replaced closing bool with atomic.Bool to eliminate mutex overhead for the closing flag
  • Updated recvLoop() to suppress error logging when the connection is closed intentionally or receives an EOF
  • Modified the Close() method to use atomic store operation instead of mutex-protected write

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
session.go Converted closing flag to atomic.Bool, updated error logging in recvLoop to check closing state, and simplified Close() method to use atomic operations
coverage.out Added test coverage output file (appears to be unintentionally committed)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR improves error logging during connection closure by suppressing warnings and errors when a session is closed gracefully. The implementation changes the `closing` field from a mutex-protected boolean to an `atomic.Bool` for cleaner concurrency control and reduced lock contention. **Key Changes:** - Replaced `closing bool` with `atomic.Bool` to eliminate mutex overhead for the closing flag - Updated `recvLoop()` to suppress error logging when the connection is closed intentionally or receives an EOF - Modified the `Close()` method to use atomic store operation instead of mutex-protected write ### Reviewed changes Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment. | File | Description | | ---- | ----------- | | session.go | Converted closing flag to atomic.Bool, updated error logging in recvLoop to check closing state, and simplified Close() method to use atomic operations | | coverage.out | Added test coverage output file (appears to be unintentionally committed) | --- 💡 <a href="/nemith/netconf/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-01-07 17:27:14 +00:00

The coverage.out file appears to be a generated test coverage artifact that should not be committed to the repository. Consider adding it to .gitignore (note that .gitignore already contains cover.out, but not coverage.out). Generated files like test coverage reports should typically be excluded from version control.

The coverage.out file appears to be a generated test coverage artifact that should not be committed to the repository. Consider adding it to .gitignore (note that .gitignore already contains cover.out, but not coverage.out). Generated files like test coverage reports should typically be excluded from version control.
Sign in to join this conversation.
No description provided.