Return errors instead of printing to logs

[apoorvajagtap]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Go Version Update
• Dependency Update

Description

With #840 debug messages were printed to stderr, as this wasn't very application friendly. With this, the noise in the logs will be reduced, without impacting error handling.

Related Tickets & Documents

• Closes [BUG] 1.5.1 uses log.Printf #880

Added/updated tests?

• Yes
• No, and this is why: current tests does not cover the scenario of stderr logs, so no impact on tests with the changes.
• I need help with writing tests

Run verifications and test

• make verify is passing
• make test is passing

[codecov]

Codecov Report

Attention: Patch coverage is 36.36364% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 73.26%. Comparing base (e5f1a0a) to head (c8a6785).

❗ Current head c8a6785 differs from pull request most recent head cc16497. Consider uploading reports for the commit cc16497 to get more accurate results

Files	Patch %	Lines
server.go	0.00%	7 Missing ⚠️
proxy.go	0.00%	5 Missing ⚠️
client.go	83.33%	1 Missing ⚠️
conn.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
+ Coverage   71.60%   73.26%   +1.66%     
==========================================
  Files          11       11              
  Lines        1585     1575      -10     
==========================================
+ Hits         1135     1154      +19     
+ Misses        345      320      -25     
+ Partials      105      101       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ghost]

With one exception, the logged errors are the result of calling a network connection Close() method on an error path. It's reasonable to ignore these close errors for two reasons:

• The functions are returning a more important error.
• There's nothing that the application can do with the information in the close error.

I would remove the calls to log.Printf by undoing the changes in 666c197.

If you are going to return the close error with errors.Join, then the secondary close error should come last in the list of errors. This does do things: the primary comes first in the error string; search on the error tree visits the primary error first.

The one exception is the error returned from bw.Flush(). That call should never return an error. It is correct to add the call to panic.

[AlexVulaj]

This looks good to me. @greenmarmot as you've been extremely helpful with these changes, do you see any further issues with this PR as it is? If not, I'd like to go ahead and merge it.