refactor(recovery): smart error comparison

[zeek0x]

Hi :)

While casually looking through the recover.go code, I noticed some logic that identifies errors by matching strings within the error message. Switching to direct error comparisons can improve the maintainability of the code.

[zeek0x]

I noticed a missed variable name rename, so I fixed it and force-pushed 🙇

[appleboy]

Please write unit testing.

[Copilot]

Pull Request Overview

This PR refactors the panic recovery logic in recovery.go to use direct errors.Is comparisons for broken pipe and connection reset errors instead of substring matching, improving maintainability.

• Replace substring-based detection of broken connections with errors.Is against syscall.EPIPE and syscall.ECONNRESET.
• Rename variables for clarity (err→rec, brokenPipe→isBrokenPipeOrConnReset).
• Remove deprecated net.OpError branch and add syscall import.

Comments suppressed due to low confidence (2)

recovery.go:61

• [nitpick] The variable name isBrokenPipeOrConnReset is quite long; consider shortening it (e.g., brokenConn) for readability.

var isBrokenPipeOrConnReset bool

recovery.go:64

• The errors package is used here but not imported; add import "errors" to the import block to avoid a compile error.

isBrokenPipeOrConnReset = errors.Is(err, syscall.EPIPE) || errors.Is(err, syscall.ECONNRESET)

[zeek0x]

@appleboy

Please write unit testing.

There was already a good unit test for this fix, so I changed it to fit better.

[Copilot]

Pull Request Overview

This PR refactors the recovery logic by switching from string matching to direct error comparisons, improving maintainability and clarity.

• Updated tests to verify error conditions using syscall.Errno comparisons.
• Revised the panic recovery handler to compare errors with errors.Is and log accordingly.

Reviewed Changes

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

File	Description
recovery_test.go	Updated test assertions to directly compare syscall.Errno error values.
recovery.go	Refactored panic recovery logic to compare errors using errors.Is with syscall errors.

[Copilot]

Consider explicitly handling cases where recover() returns a non-error value to clarify the expected type and avoid potential issues when rec is not an error.

Suggested change

	err, ok := rec.(error)
	if ok {
	isBrokenPipeOrConnReset = errors.Is(err, syscall.EPIPE) || errors.Is(err, syscall.ECONNRESET)
	var err error
	switch v := rec.(type) {
	case error:
	err = v
	isBrokenPipeOrConnReset = errors.Is(err, syscall.EPIPE) || errors.Is(err, syscall.ECONNRESET)
	default:
	err = fmt.Errorf("panic of unknown type: %v", v)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.92%. Comparing base (3dc1cd6) to head (4732fa7).
Report is 129 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4142      +/-   ##
==========================================
- Coverage   99.21%   98.92%   -0.30%     
==========================================
  Files          42       44       +2     
  Lines        3182     3432     +250     
==========================================
+ Hits         3157     3395     +238     
- Misses         17       26       +9     
- Partials        8       11       +3     

Flag	Coverage Δ
	?
--ldflags="-checklinkname=0" -tags sonic	98.85% <100.00%> (?)
-tags go_json	98.85% <100.00%> (?)
-tags nomsgpack	98.90% <100.00%> (?)
go-1.18	?
go-1.19	?
go-1.20	?
go-1.21	?
go-1.23	98.92% <100.00%> (?)
go-1.24	98.92% <100.00%> (?)
macos-latest	98.92% <100.00%> (-0.30%)	⬇️
ubuntu-latest	98.92% <100.00%> (-0.30%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[zeek0x]

It looks like coverage dropped because the file got shorter, not because there's more untested code.