Skip to content

Commit

Permalink
[serverless/proxy]: override default ErrorHandler
Browse files Browse the repository at this point in the history
The default error handler logs "http: proxy error: %v" then returns an HTTP 502 (bad gateway)
response. This is unfortunate because it lacks much any context on the original request that
failed, and the commonly observed error today is "context deadline exceeded", which is not
actionnable if you don't know what request it was for. It also logs to STDERR and does not
honor the agent's log level.

To address this, we introduce a custom error handler function that logs the error with some
relevant dynamic context using `log.Debugf`, and returns an HTTP 502 and HTTP 504 response
depending on whether the error is a `context.DeadlineExceeded` error or not.
  • Loading branch information
RomainMuller committed Dec 18, 2024
1 parent 48763aa commit ef8b8af
Showing 1 changed file with 37 additions and 1 deletion.
38 changes: 37 additions & 1 deletion pkg/serverless/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package proxy

import (
"context"
"errors"
"net/http"
"net/http/httputil"
"net/url"
Expand Down Expand Up @@ -68,9 +69,44 @@ func newProxy(target string, processor invocationlifecycle.InvocationProcessor)
Scheme: "http",
Host: target,
}
proxy := httputil.NewSingleHostReverseProxy(url)

// The default error handler logs "http: proxy error: %v" then returns an HTTP 502 (bad gateway)
// response. This is unfortunate because it lacks much any context on the original request that
// failed, and the commonly observed error today is "context deadline exceeded", which is not
// actionnable if you don't know what request it was for. It also logs to STDERR and does not
// honor the agent's log level.
proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) {
log.Debugf(
"[serverless/proxy][%T] %s %s -- proxy error: %v",
// The dynamic type of processor informs about what kind of proxy this was (main/appsec)
processor,
// The request method and URL are useful to understand what exactly failed. We won't log
// the body (too large) or headers (risks containing sensitive data, such as API keys)
r.Method, r.URL,
// What happened that caused us to be called?
err,
)

// If the error is a [context.DeadlineExceeded], we return an HTTP 504 (gateway timeout)
// instead of the generic HTTP 502 (bad gateway) to give the client a better idea of what is
// going on (this may influence retry behavior, for example).
if errors.Is(err, context.DeadlineExceeded) {
w.WriteHeader(http.StatusGatewayTimeout)
} else {
// Return an HTTP 502 (bad gateway) error response; defer the retrying to the client.
w.WriteHeader(http.StatusBadGateway)
}

// Writing the error message as best-effort, we simply debug-log any error that occur here.
if _, err := w.Write([]byte(err.Error())); err != nil {
log.Debugf("[serverless/proxy][%T] failed to write error message to response body: %w", err)
}
}

return &runtimeProxy{
target: url,
proxy: httputil.NewSingleHostReverseProxy(url),
proxy: proxy,
processor: processor,
}
}

0 comments on commit ef8b8af

Please sign in to comment.