From 821efe206a1a31d6d228be69f5fe8c1118ffca95 Mon Sep 17 00:00:00 2001 From: William Petit Date: Fri, 27 Sep 2024 10:09:25 +0200 Subject: [PATCH] feat: global error handler with template rendering --- internal/proxy/director/context.go | 22 +++++++++++++ internal/proxy/director/director.go | 18 ++++++----- internal/proxy/director/layer/authn/layer.go | 20 ++++++------ .../layer/authn/oidc/authenticator.go | 2 +- .../proxy/director/layer/authn/oidc/client.go | 4 +-- internal/proxy/director/layer/queue/queue.go | 28 ++++++++--------- .../proxy/director/layer/rewriter/layer.go | 8 ++--- internal/proxy/director/options.go | 19 ++++++++++-- internal/proxy/server.go | 31 +++++++++++-------- templates/error.gohtml | 16 ++++++++-- 10 files changed, 109 insertions(+), 59 deletions(-) diff --git a/internal/proxy/director/context.go b/internal/proxy/director/context.go index 9b97c3f..62200e7 100644 --- a/internal/proxy/director/context.go +++ b/internal/proxy/director/context.go @@ -2,10 +2,12 @@ package director import ( "context" + "net/http" "net/url" "forge.cadoles.com/cadoles/bouncer/internal/store" "github.com/pkg/errors" + "gitlab.com/wpetit/goweb/logger" ) type contextKey string @@ -14,6 +16,7 @@ const ( contextKeyProxy contextKey = "proxy" contextKeyLayers contextKey = "layers" contextKeyOriginalURL contextKey = "originalURL" + contextKeyHandleError contextKey = "handleError" ) var ( @@ -60,3 +63,22 @@ func ctxValue[T any](ctx context.Context, key contextKey) (T, error) { return value, nil } + +type HandleErrorFunc func(w http.ResponseWriter, r *http.Request, status int, err error) + +func withHandleError(ctx context.Context, fn HandleErrorFunc) context.Context { + return context.WithValue(ctx, contextKeyHandleError, fn) +} + +func HandleError(ctx context.Context, w http.ResponseWriter, r *http.Request, status int, err error) { + err = errors.WithStack(err) + + fn, ok := ctx.Value(contextKeyHandleError).(HandleErrorFunc) + if !ok { + logger.Error(ctx, err.Error(), logger.CapturedE(err)) + http.Error(w, http.StatusText(status), status) + return + } + + fn(w, r, status, err) +} diff --git a/internal/proxy/director/director.go b/internal/proxy/director/director.go index 7293b06..fb1c6c1 100644 --- a/internal/proxy/director/director.go +++ b/internal/proxy/director/director.go @@ -22,6 +22,8 @@ type Director struct { proxyCache cache.Cache[string, []*store.Proxy] layerCache cache.Cache[string, []*store.Layer] + + handleError HandleErrorFunc } func (d *Director) rewriteRequest(r *http.Request) (*http.Request, error) { @@ -33,7 +35,6 @@ func (d *Director) rewriteRequest(r *http.Request) (*http.Request, error) { } url := getRequestURL(r) - ctx = withOriginalURL(ctx, url) ctx = logger.With(ctx, logger.F("url", url.String())) @@ -169,6 +170,7 @@ func (d *Director) getLayers(ctx context.Context, proxyName store.ProxyName) ([] func (d *Director) RequestTransformer() proxy.RequestTransformer { return func(r *http.Request) { ctx := r.Context() + layers, err := ctxLayers(ctx) if err != nil { if errors.Is(err, errContextKeyNotFound) { @@ -225,15 +227,16 @@ func (d *Director) ResponseTransformer() proxy.ResponseTransformer { func (d *Director) Middleware() proxy.Middleware { return func(next http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { + ctx := withHandleError(r.Context(), d.handleError) + r = r.WithContext(ctx) + r, err := d.rewriteRequest(r) if err != nil { - logger.Error(r.Context(), "could not rewrite request", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + HandleError(ctx, w, r, http.StatusInternalServerError, errors.Wrap(err, "could not rewrite request")) return } - ctx := r.Context() + ctx = r.Context() layers, err := ctxLayers(ctx) if err != nil { @@ -241,9 +244,7 @@ func (d *Director) Middleware() proxy.Middleware { return } - logger.Error(ctx, "could not retrieve proxy and layers from context", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + HandleError(ctx, w, r, http.StatusInternalServerError, errors.Wrap(err, "could not retrieve proxy and layers from context")) return } @@ -278,5 +279,6 @@ func New(proxyRepository store.ProxyRepository, layerRepository store.LayerRepos layerRegistry: registry, proxyCache: opts.ProxyCache, layerCache: opts.LayerCache, + handleError: opts.HandleError, } } diff --git a/internal/proxy/director/layer/authn/layer.go b/internal/proxy/director/layer/authn/layer.go index cd5e635..7de6ce2 100644 --- a/internal/proxy/director/layer/authn/layer.go +++ b/internal/proxy/director/layer/authn/layer.go @@ -1,7 +1,9 @@ package authn import ( + "bytes" "html/template" + "io" "net/http" "path/filepath" @@ -29,9 +31,7 @@ func (l *Layer) Middleware(layer *store.Layer) proxy.Middleware { options, err := fromStoreOptions(layer.Options) if err != nil { - logger.Error(ctx, "could not parse layer options", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.Wrap(err, "could not parse layer options")) return } @@ -162,20 +162,22 @@ func (l *Layer) renderPage(w http.ResponseWriter, r *http.Request, page string, tmpl, err := template.New("").Funcs(sprig.FuncMap()).ParseGlob(pattern) if err != nil { - logger.Error(ctx, "could not load authn templates", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.Wrap(err, "could not load authn templates")) return } w.Header().Add("Cache-Control", "no-cache") - if err := tmpl.ExecuteTemplate(w, block, templateData); err != nil { - logger.Error(ctx, "could not render authn page", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + var buf bytes.Buffer + if err := tmpl.ExecuteTemplate(w, block, templateData); err != nil { + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.Wrap(err, "could not render authn page")) return } + + if _, err := io.Copy(w, &buf); err != nil { + logger.Error(ctx, "could not write authn page", logger.CapturedE(errors.WithStack(err))) + } } // LayerType implements director.MiddlewareLayer diff --git a/internal/proxy/director/layer/authn/oidc/authenticator.go b/internal/proxy/director/layer/authn/oidc/authenticator.go index 6693bee..ea1e1e6 100644 --- a/internal/proxy/director/layer/authn/oidc/authenticator.go +++ b/internal/proxy/director/layer/authn/oidc/authenticator.go @@ -86,7 +86,7 @@ func (a *Authenticator) PreAuthentication(w http.ResponseWriter, r *http.Request if postLogoutRedirectURL != "" { isAuthorized := slices.Contains(options.OIDC.PostLogoutRedirectURLs, postLogoutRedirectURL) if !isAuthorized { - http.Error(w, "unauthorized post-logout redirect", http.StatusBadRequest) + director.HandleError(ctx, w, r, http.StatusBadRequest, errors.New("unauthorized post-logout redirect")) return errors.WithStack(authn.ErrSkipRequest) } } diff --git a/internal/proxy/director/layer/authn/oidc/client.go b/internal/proxy/director/layer/authn/oidc/client.go index 11f9fac..90c79c3 100644 --- a/internal/proxy/director/layer/authn/oidc/client.go +++ b/internal/proxy/director/layer/authn/oidc/client.go @@ -6,6 +6,7 @@ import ( "net/url" "strings" + "forge.cadoles.com/cadoles/bouncer/internal/proxy/director" "github.com/coreos/go-oidc/v3/oidc" "github.com/dchest/uniuri" "github.com/gorilla/sessions" @@ -68,8 +69,7 @@ func (c *Client) login(w http.ResponseWriter, r *http.Request, sess *sessions.Se sess.Values[sessionKeyPostLoginRedirectURL] = postLoginRedirectURL if err := sess.Save(r, w); err != nil { - logger.Error(ctx, "could not save session", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("could not save session")) return } diff --git a/internal/proxy/director/layer/queue/queue.go b/internal/proxy/director/layer/queue/queue.go index c638578..e07638e 100644 --- a/internal/proxy/director/layer/queue/queue.go +++ b/internal/proxy/director/layer/queue/queue.go @@ -1,9 +1,11 @@ package queue import ( + "bytes" "context" "fmt" "html/template" + "io" "math/rand" "net/http" "path/filepath" @@ -52,9 +54,7 @@ func (q *Queue) Middleware(layer *store.Layer) proxy.Middleware { options, err := fromStoreOptions(layer.Options, q.defaultKeepAlive) if err != nil { - logger.Error(ctx, "could not parse layer options", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("could not parse layer options")) return } @@ -89,9 +89,7 @@ func (q *Queue) Middleware(layer *store.Layer) proxy.Middleware { rank, err := q.adapter.Touch(ctx, queueName, sessionID) if err != nil { - logger.Error(ctx, "could not retrieve session rank", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("could not retrieve session rank")) return } @@ -144,9 +142,7 @@ func (q *Queue) renderQueuePage(w http.ResponseWriter, r *http.Request, queueNam status, err := q.adapter.Status(ctx, queueName) if err != nil { - logger.Error(ctx, "could not retrieve queue status", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("could not retrieve queue status")) return } @@ -166,9 +162,7 @@ func (q *Queue) renderQueuePage(w http.ResponseWriter, r *http.Request, queueNam }) if q.tmpl == nil { - logger.Error(ctx, "queue page templates not loaded", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("queue page templates not loaded")) return } @@ -194,12 +188,16 @@ func (q *Queue) renderQueuePage(w http.ResponseWriter, r *http.Request, queueNam w.Header().Add("Retry-After", strconv.FormatInt(int64(refreshRate.Seconds()), 10)) w.WriteHeader(http.StatusServiceUnavailable) - if err := q.tmpl.ExecuteTemplate(w, "queue", templateData); err != nil { - logger.Error(ctx, "could not render queue page", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + var buf bytes.Buffer + if err := q.tmpl.ExecuteTemplate(&buf, "queue", templateData); err != nil { + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("could not render queue page")) return } + + if _, err := io.Copy(w, &buf); err != nil { + logger.Error(ctx, "could not write queue page", logger.CapturedE(errors.WithStack(err))) + } } func (q *Queue) refreshQueue(ctx context.Context, layerName store.LayerName, keepAlive time.Duration) { diff --git a/internal/proxy/director/layer/rewriter/layer.go b/internal/proxy/director/layer/rewriter/layer.go index c05bf1b..2cfc9d6 100644 --- a/internal/proxy/director/layer/rewriter/layer.go +++ b/internal/proxy/director/layer/rewriter/layer.go @@ -11,7 +11,6 @@ import ( ruleHTTP "forge.cadoles.com/cadoles/bouncer/internal/rule/http" "forge.cadoles.com/cadoles/bouncer/internal/store" "github.com/pkg/errors" - "gitlab.com/wpetit/goweb/logger" ) const LayerType store.LayerType = "rewriter" @@ -32,9 +31,7 @@ func (l *Layer) Middleware(layer *store.Layer) proxy.Middleware { options, err := fromStoreOptions(layer.Options) if err != nil { - logger.Error(ctx, "could not parse layer options", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.New("could not parse layer options")) return } @@ -52,8 +49,7 @@ func (l *Layer) Middleware(layer *store.Layer) proxy.Middleware { return } - logger.Error(ctx, "could not apply request rules", logger.CapturedE(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + director.HandleError(ctx, w, r, http.StatusInternalServerError, errors.Wrap(err, "could not apply request rules")) return } diff --git a/internal/proxy/director/options.go b/internal/proxy/director/options.go index c785aeb..cec9dbd 100644 --- a/internal/proxy/director/options.go +++ b/internal/proxy/director/options.go @@ -1,18 +1,21 @@ package director import ( + "net/http" "time" "forge.cadoles.com/cadoles/bouncer/internal/cache" "forge.cadoles.com/cadoles/bouncer/internal/cache/memory" "forge.cadoles.com/cadoles/bouncer/internal/cache/ttl" "forge.cadoles.com/cadoles/bouncer/internal/store" + "gitlab.com/wpetit/goweb/logger" ) type Options struct { - Layers []Layer - ProxyCache cache.Cache[string, []*store.Proxy] - LayerCache cache.Cache[string, []*store.Layer] + Layers []Layer + ProxyCache cache.Cache[string, []*store.Proxy] + LayerCache cache.Cache[string, []*store.Layer] + HandleError HandleErrorFunc } type OptionFunc func(opts *Options) @@ -30,6 +33,10 @@ func NewOptions(funcs ...OptionFunc) *Options { memory.NewCache[string, time.Time](), 30*time.Second, ), + HandleError: func(w http.ResponseWriter, r *http.Request, status int, err error) { + logger.Error(r.Context(), err.Error(), logger.CapturedE(err)) + http.Error(w, http.StatusText(status), status) + }, } for _, fn := range funcs { @@ -56,3 +63,9 @@ func WithLayerCache(cache cache.Cache[string, []*store.Layer]) OptionFunc { opts.LayerCache = cache } } + +func WithHandleErrorFunc(fn HandleErrorFunc) OptionFunc { + return func(opts *Options) { + opts.HandleError = fn + } +} diff --git a/internal/proxy/server.go b/internal/proxy/server.go index 3f6ef51..21984ed 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -1,9 +1,11 @@ package proxy import ( + "bytes" "context" "fmt" "html/template" + "io" "log" "net" "net/http" @@ -23,7 +25,6 @@ import ( "forge.cadoles.com/cadoles/bouncer/internal/store" "github.com/Masterminds/sprig/v3" - "github.com/getsentry/sentry-go" sentryhttp "github.com/getsentry/sentry-go/http" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" @@ -112,6 +113,7 @@ func (s *Server) run(parentCtx context.Context, addrs chan net.Addr, errs chan e s.directorCacheTTL, ), ), + director.WithHandleErrorFunc(s.handleError), ) if s.serverConfig.HTTP.UseRealIP { @@ -217,27 +219,24 @@ func (s *Server) createReverseProxy(ctx context.Context, target *url.URL) *httpu httpTransport.DialContext = dialer.DialContext reverseProxy.Transport = httpTransport - reverseProxy.ErrorHandler = s.handleError + reverseProxy.ErrorHandler = s.handleProxyError return reverseProxy } func (s *Server) handleDefault(w http.ResponseWriter, r *http.Request) { - err := errors.Errorf("no proxy target found") - - logger.Error(r.Context(), "proxy error", logger.CapturedE(err)) - sentry.CaptureException(err) - - s.renderErrorPage(w, r, err, http.StatusBadGateway, http.StatusText(http.StatusBadGateway)) + s.handleError(w, r, http.StatusBadGateway, errors.Errorf("no proxy target found")) } -func (s *Server) handleError(w http.ResponseWriter, r *http.Request, err error) { +func (s *Server) handleError(w http.ResponseWriter, r *http.Request, status int, err error) { err = errors.WithStack(err) + logger.Error(r.Context(), err.Error(), logger.CapturedE(err)) - logger.Error(r.Context(), "proxy error", logger.CapturedE(err)) - sentry.CaptureException(err) + s.renderErrorPage(w, r, err, status, http.StatusText(status)) +} - s.renderErrorPage(w, r, err, http.StatusBadGateway, http.StatusText(http.StatusBadGateway)) +func (s *Server) handleProxyError(w http.ResponseWriter, r *http.Request, err error) { + s.handleError(w, r, http.StatusBadGateway, err) } func (s *Server) renderErrorPage(w http.ResponseWriter, r *http.Request, err error, statusCode int, status string) { @@ -288,12 +287,18 @@ func (s *Server) renderPage(w http.ResponseWriter, r *http.Request, page string, return } - if err := blockTmpl.Execute(w, templateData); err != nil { + var buf bytes.Buffer + + if err := blockTmpl.Execute(&buf, templateData); err != nil { logger.Error(ctx, "could not render proxy page", logger.CapturedE(errors.WithStack(err))) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } + + if _, err := io.Copy(w, &buf); err != nil { + logger.Error(ctx, "could not write page", logger.CapturedE(errors.WithStack(err))) + } } func NewServer(funcs ...OptionFunc) *Server { diff --git a/templates/error.gohtml b/templates/error.gohtml index 9e4c9ae..39f72cb 100644 --- a/templates/error.gohtml +++ b/templates/error.gohtml @@ -76,14 +76,26 @@ margin-top: 2em; text-align: right; } + + .stacktrace { + max-height: 250px; + overflow-y: auto; + background-color: #dca0a0; + padding: 0 10px; + border-radius: 5px; + margin-top: 10px; + }
-

{{ $title }}

+

⚠ {{ $title }}

{{ if .Debug }} - 
{{ .Err }}
+

Stack Trace

+
+ 
{{ printf "%+v" .Err }}
+
{{ end }}

Propulsé par