fix: add logging when closing transport #127

Merged

nemith merged 1 commit from brb/push-ttkpqspmkopy into main

2026-01-07 18:31:17 +00:00

nemith commented

2026-01-07 18:25:55 +00:00

(Migrated from github.com)

Some missing error handling for closing streams to make sure we capture these.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-01-07 18:29:06 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This pull request improves error handling by adding logging when closing transport and message reader resources, replacing previous nolint:errcheck and TODO comments with actual error capture and logging.

Adds error handling and logging when closing transport after handshake failure
Adds error handling and logging when closing message readers in defer statements

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

## Pull request overview This pull request improves error handling by adding logging when closing transport and message reader resources, replacing previous `nolint:errcheck` and `TODO` comments with actual error capture and logging. - Adds error handling and logging when closing transport after handshake failure - Adds error handling and logging when closing message readers in defer statements --- 💡 <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>.

session.go

For consistency with the change at line 186 which uses closeErr to avoid variable shadowing, consider using a different variable name here (e.g., closeErr) instead of err. While the shadowing doesn't cause a functional issue here since the scopes are separate, using a distinct name improves code clarity and maintains consistency with the naming convention established elsewhere in this same change.

		if closeErr := r.Close(); closeErr != nil {
			s.logger.Warn("failed to close message reader", "error", closeErr)

For consistency with the change at line 186 which uses `closeErr` to avoid variable shadowing, consider using a different variable name here (e.g., `closeErr`) instead of `err`. While the shadowing doesn't cause a functional issue here since the scopes are separate, using a distinct name improves code clarity and maintains consistency with the naming convention established elsewhere in this same change. ```suggestion if closeErr := r.Close(); closeErr != nil { s.logger.Warn("failed to close message reader", "error", closeErr) ```