mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-06 22:32:16 +00:00
fix: recover from request panics (#11713)
# Which Problems Are Solved Panics may cause a service discruption by unexpectedly closing an request's connection. Or in the case of gRPC completely killing the service. Allthough panics are still individual bugs that need to be solved, this PR makes sure a panic is gracefully handled and an understandable error is returned to the client. # How the Problems Are Solved - Recover in the middleware interceptors for the 3 API protocols (HTTP, gRPC, connect). - HTTP middleware uses formatted responses for: - OIDC errors (JSON formatted response) - UI (error page rendering) - SCIM - Upon recovery an alert level log is printed (ERROR+4 for stdlib log handlers) # Additional Context - internal observation --------- Co-authored-by: Livio Spring <[email protected]> Co-authored-by: Livio Spring <[email protected]>
This commit is contained in:
co-authored by
Livio Spring
Livio Spring
parent
bebee4c795
commit
5934e07960
@@ -0,0 +1,82 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/zitadel/sloggcp"
|
||||
|
||||
"github.com/zitadel/zitadel/backend/v3/instrumentation/logging"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
// RecoverHandler recovers from panics in the HTTP handler chain
|
||||
// and calls the provided writeResponse function to write an appropriate response to the client.
|
||||
//
|
||||
// The request context is canceled with the panic error as the cause.
|
||||
func RecoverHandler(writeResponse func(w http.ResponseWriter, r *http.Request, err error)) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
ctx, cancel := context.WithCancelCause(r.Context())
|
||||
r = r.WithContext(ctx)
|
||||
|
||||
defer func() {
|
||||
var err error
|
||||
if rec := recover(); rec != nil {
|
||||
recErr, ok := rec.(error)
|
||||
if !ok {
|
||||
recErr = fmt.Errorf("%v", rec)
|
||||
}
|
||||
err = zerrors.ThrowInternal(recErr, zerrors.IDRecover, "Errors.Internal")
|
||||
logRecovered(ctx, err)
|
||||
writeResponse(w, r, err)
|
||||
}
|
||||
cancel(err)
|
||||
}()
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// FallbackRecoverHandler recovers from panics in the HTTP handler chain
|
||||
// and returns a 500 Internal Server Error response.
|
||||
// The request context is canceled with the panic error as the cause,
|
||||
// so that any ongoing operations can be stopped and cleaned up.
|
||||
//
|
||||
// The response is sent as a text/plain response.
|
||||
// It is used as a last line of defense to prevent the server from crashing
|
||||
// due to panics in the handlers.
|
||||
// Protocols (OIDC, SAML, HTML etc.) should use [RecoverHandler] to write
|
||||
// properly formatted error responses to the clients.
|
||||
func FallbackRecoverHandler() func(http.Handler) http.Handler {
|
||||
return RecoverHandler(func(w http.ResponseWriter, r *http.Request, _ error) {
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
})
|
||||
}
|
||||
|
||||
// RecoverHandlerWithError is similar to [RecoverHandler] but returns an error instead of writing a response directly.
|
||||
func RecoverHandlerWithError(next HandlerFuncWithError) HandlerFuncWithError {
|
||||
return func(w http.ResponseWriter, r *http.Request) (err error) {
|
||||
ctx, cancel := context.WithCancelCause(r.Context())
|
||||
r = r.WithContext(ctx)
|
||||
defer func() {
|
||||
if rec := recover(); rec != nil {
|
||||
recErr, ok := rec.(error)
|
||||
if !ok {
|
||||
recErr = fmt.Errorf("%v", rec)
|
||||
}
|
||||
err = zerrors.ThrowInternal(recErr, zerrors.IDRecover, "Errors.Internal")
|
||||
logRecovered(ctx, err)
|
||||
}
|
||||
cancel(err)
|
||||
}()
|
||||
return next(w, r)
|
||||
}
|
||||
}
|
||||
|
||||
func logRecovered(ctx context.Context, err error) {
|
||||
logger := logging.FromCtx(ctx)
|
||||
logger.Log(ctx, sloggcp.LevelAlert, "recovered from panic", "err", err)
|
||||
}
|
||||
@@ -0,0 +1,91 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
func TestRecoverHandler(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
handler http.HandlerFunc
|
||||
wantStatus int
|
||||
wantBody string
|
||||
}{
|
||||
{
|
||||
name: "no panic",
|
||||
handler: func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
io.WriteString(w, "ok") //nolint:errcheck
|
||||
},
|
||||
wantStatus: http.StatusOK,
|
||||
wantBody: "ok",
|
||||
},
|
||||
{
|
||||
name: "panic with error",
|
||||
handler: func(w http.ResponseWriter, r *http.Request) {
|
||||
panic(errors.New("oops"))
|
||||
},
|
||||
wantStatus: http.StatusInternalServerError,
|
||||
wantBody: "ID=RECOVER Message=Errors.Internal Parent=(oops)",
|
||||
},
|
||||
{
|
||||
name: "panic with string",
|
||||
handler: func(w http.ResponseWriter, r *http.Request) {
|
||||
panic("something went wrong")
|
||||
},
|
||||
wantStatus: http.StatusInternalServerError,
|
||||
wantBody: "ID=RECOVER Message=Errors.Internal Parent=(something went wrong)",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
handler := RecoverHandler(writeResponse)(tt.handler)
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
handler.ServeHTTP(w, r)
|
||||
|
||||
res := w.Result()
|
||||
defer res.Body.Close()
|
||||
body, _ := io.ReadAll(res.Body)
|
||||
assert.Equal(t, tt.wantStatus, res.StatusCode)
|
||||
assert.Equal(t, tt.wantBody, string(body))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFallbackRecoverHandler(t *testing.T) {
|
||||
handler := FallbackRecoverHandler()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
panic("unexpected error")
|
||||
}))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
handler.ServeHTTP(w, r)
|
||||
|
||||
res := w.Result()
|
||||
defer res.Body.Close()
|
||||
body, _ := io.ReadAll(res.Body)
|
||||
assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
|
||||
assert.Equal(t, "Internal Server Error\n", string(body))
|
||||
}
|
||||
|
||||
func TestRecoverHandlerWithError(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
// Panic on nil handler.
|
||||
err := RecoverHandlerWithError(nil)(w, r)
|
||||
assert.ErrorIs(t, err, zerrors.ThrowInternal(nil, zerrors.IDRecover, "Errors.Internal"))
|
||||
}
|
||||
|
||||
func writeResponse(w http.ResponseWriter, _ *http.Request, err error) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
io.WriteString(w, err.Error()) //nolint:errcheck
|
||||
}
|
||||
Reference in New Issue
Block a user