Skip to content

Conversation

egorikas
Copy link
Collaborator

Why
Ignoring of errors is a bad coding practice that can lead to the non-expected results. Silent errors may cause serious disturbances to the application working.

What
Fixing part of the ignored/missed errors.

How

  • Handling some errors explicitly.
  • Providing a few ignores in places, where the fix has no sense.
  • Providing a closers.Close function to log errors happening during closing of different objects.

- handling some missed errors

Signed-off-by: egorikas <[email protected]>
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves error handling by addressing ignored/missed errors throughout the codebase. The primary focus is to explicitly handle errors that were previously being silently ignored, which could lead to unexpected application behavior.

  • Introduces a new closers.Close utility function that logs errors during resource cleanup
  • Replaces direct defer closer.Close() calls with proper error handling
  • Adds explicit error checking for write operations and other critical functions

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
utils/closers/closer.go New utility for logging errors during resource cleanup
utils/closers/closer_test.go Comprehensive tests for the closer utility
mocks/io/mock_closer.go Mock implementation for testing closer functionality
Multiple client/server files Replaced bare defer statements with proper error handling using closers.Close
Test files Added error checking for write operations and resource cleanup

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

egorikas and others added 3 commits September 15, 2025 10:53
Fix spelling error: 'utulity' should be 'utility'.

Co-authored-by: Copilot <[email protected]>
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@sambhav-jain-16 sambhav-jain-16 left a comment

Choose a reason for hiding this comment

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

Thanks @egorikas
Some queries but overall looks good.

@egorikas
Copy link
Collaborator Author

Thanks @egorikas Some queries but overall looks good.

Thanks for the review. I've made a few quick fixes here and there.

Copy link
Collaborator

@sambhav-jain-16 sambhav-jain-16 left a comment

Choose a reason for hiding this comment

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

Thanks @egorikas for handing this
LGTM

@egorikas egorikas merged commit a490967 into uber:master Sep 17, 2025
9 checks passed
@egorikas egorikas deleted the fixing-lint-v4 branch September 17, 2025 14:55
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