feat(api): standardise panic responses
Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
ac59d284b1
commit
1cc0f2fd48
5 changed files with 87 additions and 2 deletions
2
api.go
2
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 {
|
||||
|
|
|
|||
45
api_test.go
45
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) {
|
||||
|
|
|
|||
|
|
@ -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 <token> header.
|
||||
// Requests to paths in the skip list are allowed through without authentication.
|
||||
// Returns 401 with Fail("unauthorised", ...) on missing or invalid tokens.
|
||||
|
|
|
|||
18
openapi.go
18
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()),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue