diff --git a/api.go b/api.go index 171274c..5e58dce 100644 --- a/api.go +++ b/api.go @@ -150,7 +150,7 @@ func (e *Engine) Serve(ctx context.Context) error { // user-supplied middleware, the health endpoint, and all registered route groups. func (e *Engine) build() *gin.Engine { r := gin.New() - r.Use(gin.Recovery()) + r.Use(recoveryMiddleware()) // Apply user-supplied middleware after recovery but before routes. for _, mw := range e.middlewares { diff --git a/api_test.go b/api_test.go index 3198c8c..b101d6b 100644 --- a/api_test.go +++ b/api_test.go @@ -29,6 +29,16 @@ func (h *healthGroup) RegisterRoutes(rg *gin.RouterGroup) { }) } +type panicGroup struct{} + +func (p *panicGroup) Name() string { return "panic" } +func (p *panicGroup) BasePath() string { return "/panic" } +func (p *panicGroup) RegisterRoutes(rg *gin.RouterGroup) { + rg.GET("/boom", func(c *gin.Context) { + panic("boom") + }) +} + // ── New ───────────────────────────────────────────────────────────────── func TestNew_Good(t *testing.T) { @@ -149,6 +159,41 @@ func TestHandler_Bad_NotFound(t *testing.T) { } } +func TestHandler_Bad_PanicReturnsEnvelope(t *testing.T) { + gin.SetMode(gin.TestMode) + e, _ := api.New(api.WithRequestID()) + e.Register(&panicGroup{}) + + h := e.Handler() + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/panic/boom", nil) + h.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d", w.Code) + } + + var resp api.Response[any] + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal error: %v", err) + } + if resp.Success { + t.Fatal("expected Success=false") + } + if resp.Error == nil { + t.Fatal("expected Error to be non-nil") + } + if resp.Error.Code != "internal_server_error" { + t.Fatalf("expected error code=%q, got %q", "internal_server_error", resp.Error.Code) + } + if resp.Error.Message != "Internal server error" { + t.Fatalf("expected error message=%q, got %q", "Internal server error", resp.Error.Message) + } + if got := w.Header().Get("X-Request-ID"); got == "" { + t.Fatal("expected X-Request-ID header to survive panic recovery") + } +} + // ── Serve + graceful shutdown ─────────────────────────────────────────── func TestServe_Good_GracefulShutdown(t *testing.T) { diff --git a/middleware.go b/middleware.go index e14d3f6..bbd50f5 100644 --- a/middleware.go +++ b/middleware.go @@ -5,7 +5,9 @@ package api import ( "crypto/rand" "encoding/hex" + "fmt" "net/http" + "runtime/debug" "strings" "time" @@ -19,6 +21,20 @@ const requestIDContextKey = "request_id" // calculate elapsed duration for response metadata. const requestStartContextKey = "request_start" +// recoveryMiddleware converts panics into a standard JSON error envelope. +// This keeps internal failures consistent with the rest of the framework +// and avoids Gin's default plain-text 500 response. +func recoveryMiddleware() gin.HandlerFunc { + return gin.CustomRecovery(func(c *gin.Context, recovered any) { + fmt.Fprintf(gin.DefaultErrorWriter, "[Recovery] panic recovered: %v\n", recovered) + debug.PrintStack() + c.AbortWithStatusJSON(http.StatusInternalServerError, Fail( + "internal_server_error", + "Internal server error", + )) + }) +} + // bearerAuthMiddleware validates the Authorization: Bearer header. // Requests to paths in the skip list are allowed through without authentication. // Returns 401 with Fail("unauthorised", ...) on missing or invalid tokens. diff --git a/openapi.go b/openapi.go index c455d8a..0dc0efc 100644 --- a/openapi.go +++ b/openapi.go @@ -231,6 +231,15 @@ func operationResponses(dataSchema map[string]any) map[string]any { }, "headers": mergeHeaders(standardResponseHeaders(), rateLimitSuccessHeaders()), }, + "500": map[string]any{ + "description": "Internal server error", + "content": map[string]any{ + "application/json": map[string]any{ + "schema": envelopeSchema(nil), + }, + }, + "headers": mergeHeaders(standardResponseHeaders(), rateLimitSuccessHeaders()), + }, } } @@ -265,6 +274,15 @@ func healthResponses() map[string]any { }, "headers": mergeHeaders(standardResponseHeaders(), rateLimitSuccessHeaders()), }, + "500": map[string]any{ + "description": "Internal server error", + "content": map[string]any{ + "application/json": map[string]any{ + "schema": envelopeSchema(nil), + }, + }, + "headers": mergeHeaders(standardResponseHeaders(), rateLimitSuccessHeaders()), + }, } } diff --git a/openapi_test.go b/openapi_test.go index f1e823e..7e4972b 100644 --- a/openapi_test.go +++ b/openapi_test.go @@ -68,6 +68,9 @@ func TestSpecBuilder_Good_EmptyGroups(t *testing.T) { if _, ok := healthResponses["504"]; !ok { t.Fatal("expected 504 response on /health") } + if _, ok := healthResponses["500"]; !ok { + t.Fatal("expected 500 response on /health") + } rateLimit429 := healthResponses["429"].(map[string]any) headers := rateLimit429["headers"].(map[string]any) if _, ok := headers["Retry-After"]; !ok { @@ -275,6 +278,9 @@ func TestSpecBuilder_Good_SecuredResponses(t *testing.T) { if _, ok := responses["504"]; !ok { t.Fatal("expected 504 response in secured operation") } + if _, ok := responses["500"]; !ok { + t.Fatal("expected 500 response in secured operation") + } rateLimit429 := responses["429"].(map[string]any) headers := rateLimit429["headers"].(map[string]any) if _, ok := headers["Retry-After"]; !ok { @@ -292,7 +298,7 @@ func TestSpecBuilder_Good_SecuredResponses(t *testing.T) { if _, ok := headers["X-RateLimit-Reset"]; !ok { t.Fatal("expected X-RateLimit-Reset header in secured operation 429 response") } - for _, code := range []string{"400", "401", "403", "504"} { + for _, code := range []string{"400", "401", "403", "504", "500"} { resp := responses[code].(map[string]any) respHeaders := resp["headers"].(map[string]any) if _, ok := respHeaders["X-Request-ID"]; !ok {