From 65639e876cfcda0cb7986b2b97d6424c857ca62d Mon Sep 17 00:00:00 2001 From: OHZEKI Naoki <0h23k1.n40k1@gmail.com> Date: Fri, 10 Oct 2025 21:42:55 +0900 Subject: [PATCH 1/3] refactor(recovery): rename var in CustomRecoveryWithWriter --- recovery.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/recovery.go b/recovery.go index fdd463f3..06da69e8 100644 --- a/recovery.go +++ b/recovery.go @@ -54,38 +54,38 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc { } return func(c *Context) { defer func() { - if err := recover(); err != nil { + if rec := recover(); rec != nil { // Check for a broken connection, as it is not really a // condition that warrants a panic stack trace. - var brokenPipe bool - if ne, ok := err.(*net.OpError); ok { + var isBrokenPipeOrConnReset bool + if ne, ok := rec.(*net.OpError); ok { var se *os.SyscallError if errors.As(ne, &se) { seStr := strings.ToLower(se.Error()) if strings.Contains(seStr, "broken pipe") || strings.Contains(seStr, "connection reset by peer") { - brokenPipe = true + isBrokenPipeOrConnReset = true } } } if logger != nil { const stackSkip = 3 - if brokenPipe { - logger.Printf("%s\n%s%s", err, secureRequestDump(c.Request), reset) + if isBrokenPipeOrConnReset { + logger.Printf("%s\n%s%s", rec, secureRequestDump(c.Request), reset) } else if IsDebugging() { logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s", - timeFormat(time.Now()), secureRequestDump(c.Request), err, stack(stackSkip), reset) + timeFormat(time.Now()), secureRequestDump(c.Request), rec, stack(stackSkip), reset) } else { logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s", - timeFormat(time.Now()), err, stack(stackSkip), reset) + timeFormat(time.Now()), rec, stack(stackSkip), reset) } } - if brokenPipe { + if isBrokenPipeOrConnReset { // If the connection is dead, we can't write a status to it. - c.Error(err.(error)) //nolint: errcheck + c.Error(rec.(error)) //nolint: errcheck c.Abort() } else { - handle(c, err) + handle(c, rec) } } }() From 6cfc3cd0fc1234755e2def6df0345fd8d4bba8a4 Mon Sep 17 00:00:00 2001 From: OHZEKI Naoki <0h23k1.n40k1@gmail.com> Date: Fri, 10 Oct 2025 21:56:35 +0900 Subject: [PATCH 2/3] refactor(recovery): smart error comparison --- recovery.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/recovery.go b/recovery.go index 06da69e8..85cb0411 100644 --- a/recovery.go +++ b/recovery.go @@ -10,12 +10,12 @@ import ( "fmt" "io" "log" - "net" "net/http" "net/http/httputil" "os" "runtime" "strings" + "syscall" "time" "github.com/gin-gonic/gin/internal/bytesconv" @@ -58,15 +58,9 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc { // Check for a broken connection, as it is not really a // condition that warrants a panic stack trace. var isBrokenPipeOrConnReset bool - if ne, ok := rec.(*net.OpError); ok { - var se *os.SyscallError - if errors.As(ne, &se) { - seStr := strings.ToLower(se.Error()) - if strings.Contains(seStr, "broken pipe") || - strings.Contains(seStr, "connection reset by peer") { - isBrokenPipeOrConnReset = true - } - } + err, ok := rec.(error) + if ok { + isBrokenPipeOrConnReset = errors.Is(err, syscall.EPIPE) || errors.Is(err, syscall.ECONNRESET) } if logger != nil { const stackSkip = 3 @@ -82,7 +76,7 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc { } if isBrokenPipeOrConnReset { // If the connection is dead, we can't write a status to it. - c.Error(rec.(error)) //nolint: errcheck + c.Error(err) //nolint: errcheck c.Abort() } else { handle(c, rec) From 75b09bd1895c6047cc71ba99b45d738872bd4427 Mon Sep 17 00:00:00 2001 From: OHZEKI Naoki <0h23k1.n40k1@gmail.com> Date: Wed, 28 May 2025 20:27:54 +0900 Subject: [PATCH 3/3] test(recovery): Directly reference the syscall error string --- recovery_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/recovery_test.go b/recovery_test.go index 8a9e3475..5ce6cba8 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -113,13 +113,13 @@ func TestFunction(t *testing.T) { func TestPanicWithBrokenPipe(t *testing.T) { const expectCode = 204 - expectMsgs := map[syscall.Errno]string{ - syscall.EPIPE: "broken pipe", - syscall.ECONNRESET: "connection reset by peer", + expectErrnos := []syscall.Errno{ + syscall.EPIPE, + syscall.ECONNRESET, } - for errno, expectMsg := range expectMsgs { - t.Run(expectMsg, func(t *testing.T) { + for _, errno := range expectErrnos { + t.Run("Recovery from "+errno.Error(), func(t *testing.T) { var buf strings.Builder router := New() @@ -137,7 +137,8 @@ func TestPanicWithBrokenPipe(t *testing.T) { w := PerformRequest(router, http.MethodGet, "/recovery") // TEST assert.Equal(t, expectCode, w.Code) - assert.Contains(t, strings.ToLower(buf.String()), expectMsg) + assert.Contains(t, strings.ToLower(buf.String()), errno.Error()) + assert.NotContains(t, strings.ToLower(buf.String()), "[Recovery]") }) } }