Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions recovery/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
// wrappedMux := recovery.Middleware(mux)
// http.ListenAndServe(":8080", wrappedMux)
//
// # Logging
//
// By default panics are recovered silently. Use [WithLogger] to log
// panic details (value, stack trace, request method and path) at ERROR level:
//
// logger := logging.New()
// wrappedMux := recovery.Middleware(mux, recovery.WithLogger(logger))
//
// # Stability
//
// This package is Beta stability. The API may have minor changes before
Expand Down
44 changes: 38 additions & 6 deletions recovery/recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,53 @@
package recovery

import (
"fmt"
"log/slog"
"net/http"
"runtime/debug"
)

// config holds the resolved configuration for the recovery middleware.
type config struct {
logger *slog.Logger
}

// Option configures the recovery middleware.
type Option func(*config)

// WithLogger sets the logger used to report recovered panics.
// When a panic is recovered and a logger is configured, the middleware
// logs the panic value, stack trace, and request context at ERROR level.
func WithLogger(l *slog.Logger) Option {
return func(c *config) {
c.logger = l
}
}

// Middleware is an HTTP middleware that recovers from panics.
// When a panic occurs, it returns a 500 Internal Server Error response
// to the client, preventing the panic from crashing the server.
//
// TODO(#7): Add configurable logging support once common logging is
// established across ToolHive. Currently panics are silently recovered.
func Middleware(next http.Handler) http.Handler {
// Options can be provided to configure logging behavior. By default,
// panics are recovered silently. Use [WithLogger] to enable logging.
func Middleware(next http.Handler, opts ...Option) http.Handler {
cfg := &config{}
for _, opt := range opts {
opt(cfg)
}

return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
defer func() {
if recover() != nil {
// TODO(#7): Log panic value and stack trace
// stack := debug.Stack()
if v := recover(); v != nil {
if cfg.logger != nil {
stack := debug.Stack()
cfg.logger.ErrorContext(r.Context(), "panic recovered",
slog.String("panic", fmt.Sprintf("%v", v)),
slog.String("method", r.Method),
slog.String("path", r.URL.Path),
slog.String("stack", string(stack)),
)
}
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
}
}()
Expand Down
69 changes: 68 additions & 1 deletion recovery/recovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
package recovery

import (
"bytes"
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"

"github.com/stacklok/toolhive-core/logging"
)

func TestMiddleware_NoPanic(t *testing.T) {
Expand Down Expand Up @@ -44,7 +47,7 @@ func TestMiddleware_RecoverFromPanic(t *testing.T) {
panic("test panic")
})

// Wrap with recovery middleware
// Wrap with recovery middleware (no logger — silent recovery)
wrappedHandler := Middleware(testHandler)

// Create test request
Expand All @@ -59,6 +62,70 @@ func TestMiddleware_RecoverFromPanic(t *testing.T) {
assert.Contains(t, rec.Body.String(), "Internal Server Error")
}

func TestMiddleware_RecoverFromPanicWithLogger(t *testing.T) {
t.Parallel()

var buf bytes.Buffer
logger := logging.New(logging.WithOutput(&buf))

// Create a test handler that panics
testHandler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
panic("test panic")
})

// Wrap with recovery middleware with logger
wrappedHandler := Middleware(testHandler, WithLogger(logger))

// Create test request
req := httptest.NewRequest(http.MethodPost, "/api/resource", nil)
rec := httptest.NewRecorder()

// Execute request - should not panic
wrappedHandler.ServeHTTP(rec, req)

// Verify 500 Internal Server Error response
assert.Equal(t, http.StatusInternalServerError, rec.Code)
assert.Contains(t, rec.Body.String(), "Internal Server Error")

// Verify log output contains expected fields
logOutput := buf.String()
assert.Contains(t, logOutput, "panic recovered")
assert.Contains(t, logOutput, "test panic")
assert.Contains(t, logOutput, "POST")
assert.Contains(t, logOutput, "/api/resource")
assert.Contains(t, logOutput, "stack")
}

func TestMiddleware_NoPanicWithLogger(t *testing.T) {
t.Parallel()

var buf bytes.Buffer
logger := logging.New(logging.WithOutput(&buf))

// Create a test handler that does not panic
testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("success"))
})

// Wrap with recovery middleware with logger
wrappedHandler := Middleware(testHandler, WithLogger(logger))

// Create test request
req := httptest.NewRequest(http.MethodGet, "/test", nil)
rec := httptest.NewRecorder()

// Execute request
wrappedHandler.ServeHTTP(rec, req)

// Verify response
assert.Equal(t, http.StatusOK, rec.Code)
assert.Equal(t, "success", rec.Body.String())

// Verify no log output when no panic occurs
assert.Empty(t, buf.String())
}

func TestMiddleware_PreservesRequestContext(t *testing.T) {
t.Parallel()

Expand Down
Loading