(answer) 05/25: fix: scope JSON 500 to API routes, skip rewriting already-flushed responses

[li...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

LinkinStars pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/answer.git

commit 9234d8024630a41400282f6dab619f716551f815
Author: hhc7 <16...@users.noreply.github.com>
AuthorDate: Tue Jun 2 13:55:35 2026 +0800

    fix: scope JSON 500 to API routes, skip rewriting already-flushed responses
---
 internal/base/middleware/recovery.go      | 26 +++++++++++--
 internal/base/middleware/recovery_test.go | 63 +++++++++++++++++++++++++++----
 internal/base/server/http.go              |  5 ++-
 3 files changed, 82 insertions(+), 12 deletions(-)

diff --git a/internal/base/middleware/recovery.go b/internal/base/middleware/recovery.go
index 7e844f78..02b1cbde 100644
--- a/internal/base/middleware/recovery.go
+++ b/internal/base/middleware/recovery.go
@@ -22,6 +22,7 @@ package middleware
 import (
 	"net/http"
 	"runtime/debug"
+	"strings"
 
 	"github.com/apache/answer/internal/base/handler"
 	"github.com/apache/answer/internal/base/reason"
@@ -29,14 +30,31 @@ import (
 	"github.com/segmentfault/pacman/log"
 )
 
-func Recovery() gin.HandlerFunc {
+func Recovery(apiPrefixes ...string) gin.HandlerFunc {
 	return func(ctx *gin.Context) {
 		defer func() {
 			if err := recover(); err != nil {
 				log.Errorf("panic recovered: %v\n%s", err, debug.Stack())
-				ctx.AbortWithStatusJSON(http.StatusInternalServerError,
-					handler.NewRespBody(http.StatusInternalServerError, reason.UnknownError).TrMsg(handler.GetLangByCtx(ctx)),
-				)
+
+				// Headers/body already flushed (SSE or any streamed response).
+				// We can no longer rewrite the response cleanly; just stop the chain.
+				if ctx.Writer.Written() {
+					ctx.Abort()
+					return
+				}
+
+				path := ctx.Request.URL.Path
+				for _, p := range apiPrefixes {
+					if strings.HasPrefix(path, p) {
+						ctx.AbortWithStatusJSON(http.StatusInternalServerError,
+							handler.NewRespBody(http.StatusInternalServerError, reason.UnknownError).
+								TrMsg(handler.GetLangByCtx(ctx)),
+						)
+						return
+					}
+				}
+
+				ctx.AbortWithStatus(http.StatusInternalServerError)
 			}
 		}()
 		ctx.Next()
diff --git a/internal/base/middleware/recovery_test.go b/internal/base/middleware/recovery_test.go
index c01719fc..58dbfdb0 100644
--- a/internal/base/middleware/recovery_test.go
+++ b/internal/base/middleware/recovery_test.go
@@ -28,16 +28,17 @@ import (
 	"github.com/gin-gonic/gin"
 )
 
-func TestRecovery_Panic(t *testing.T) {
+// Panic on an API path returns the project's unified JSON 500.
+func TestRecovery_APIPathPanic(t *testing.T) {
 	gin.SetMode(gin.TestMode)
 	r := gin.New()
-	r.Use(Recovery())
-	r.GET("/panic", func(ctx *gin.Context) {
+	r.Use(Recovery("/api"))
+	r.GET("/api/panic", func(ctx *gin.Context) {
 		panic("test panic")
 	})
 
 	w := httptest.NewRecorder()
-	req, _ := http.NewRequest(http.MethodGet, "/panic", nil)
+	req, _ := http.NewRequest(http.MethodGet, "/api/panic", nil)
 	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusInternalServerError {
@@ -53,16 +54,64 @@ func TestRecovery_Panic(t *testing.T) {
 	}
 }
 
+// Panic on a non-API path returns a bare 500 with no body, so the browser can
+// render its own error page instead of showing raw JSON.
+func TestRecovery_NonAPIPathPanic(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	r := gin.New()
+	r.Use(Recovery("/api"))
+	r.GET("/page", func(ctx *gin.Context) {
+		panic("test panic")
+	})
+
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest(http.MethodGet, "/page", nil)
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusInternalServerError {
+		t.Errorf("expected 500, got %d", w.Code)
+	}
+	if w.Body.Len() != 0 {
+		t.Errorf("expected empty body for non-API path, got: %q", w.Body.String())
+	}
+}
+
+// Panic after the response has already started writing (SSE / streamed
+// responses). The middleware must not touch the response — status and body
+// already on the wire stay untouched, no JSON gets appended.
+func TestRecovery_PanicAfterResponseStarted(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	r := gin.New()
+	r.Use(Recovery("/api"))
+	r.GET("/api/stream", func(ctx *gin.Context) {
+		ctx.Writer.WriteHeader(http.StatusOK)
+		_, _ = ctx.Writer.Write([]byte("partial data"))
+		panic("test panic after write")
+	})
+
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest(http.MethodGet, "/api/stream", nil)
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected status to remain 200 (already flushed), got %d", w.Code)
+	}
+	if w.Body.String() != "partial data" {
+		t.Errorf("expected body to remain 'partial data' (no error JSON appended), got: %q", w.Body.String())
+	}
+}
+
+// Normal requests pass through unaffected.
 func TestRecovery_NoPanic(t *testing.T) {
 	gin.SetMode(gin.TestMode)
 	r := gin.New()
-	r.Use(Recovery())
-	r.GET("/ok", func(ctx *gin.Context) {
+	r.Use(Recovery("/api"))
+	r.GET("/api/ok", func(ctx *gin.Context) {
 		ctx.String(http.StatusOK, "ok")
 	})
 
 	w := httptest.NewRecorder()
-	req, _ := http.NewRequest(http.MethodGet, "/ok", nil)
+	req, _ := http.NewRequest(http.MethodGet, "/api/ok", nil)
 	r.ServeHTTP(w, req)
 
 	if w.Code != http.StatusOK {
diff --git a/internal/base/server/http.go b/internal/base/server/http.go
index 1e8204d3..8db55744 100644
--- a/internal/base/server/http.go
+++ b/internal/base/server/http.go
@@ -52,7 +52,10 @@ func NewHTTPServer(debug bool,
 		gin.SetMode(gin.ReleaseMode)
 	}
 	r := gin.New()
-	r.Use(middleware.Recovery())
+	r.Use(middleware.Recovery(
+		uiConf.APIBaseURL+"/answer/api/v1",
+		uiConf.APIBaseURL+"/answer/admin/api",
+	))
 	r.Use(func(ctx *gin.Context) {
 		if strings.Contains(ctx.Request.URL.Path, "/chat/completions") {
 			return