From 2a6859a5e46227473220c653668fa9da9f00b14e Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Tue, 10 Oct 2023 18:07:20 -0400 Subject: reverseproxy: Fix retries on "upstreams unavailable" error (#5841) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'modules/caddyhttp') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 395530d..64a7d7a 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -357,7 +357,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { // set defaults on passive health checks, if necessary if h.HealthChecks.Passive != nil { h.HealthChecks.Passive.logger = h.logger.Named("health_checker.passive") - if h.HealthChecks.Passive.FailDuration > 0 && h.HealthChecks.Passive.MaxFails == 0 { + if h.HealthChecks.Passive.MaxFails == 0 { h.HealthChecks.Passive.MaxFails = 1 } } @@ -480,7 +480,7 @@ func (h *Handler) proxyLoopIteration(r *http.Request, origReq *http.Request, w h upstream := h.LoadBalancing.SelectionPolicy.Select(upstreams, r, w) if upstream == nil { if proxyErr == nil { - proxyErr = caddyhttp.Error(http.StatusServiceUnavailable, fmt.Errorf("no upstreams available")) + proxyErr = caddyhttp.Error(http.StatusServiceUnavailable, noUpstreamsAvailable) } if !h.LoadBalancing.tryAgain(h.ctx, start, retries, proxyErr, r) { return true, proxyErr @@ -1010,17 +1010,23 @@ func (lb LoadBalancing) tryAgain(ctx caddy.Context, start time.Time, retries int // should be safe to retry, since without a connection, no // HTTP request can be transmitted; but if the error is not // specifically a dialer error, we need to be careful - if _, ok := proxyErr.(DialError); proxyErr != nil && !ok { + if proxyErr != nil { + _, isDialError := proxyErr.(DialError) + herr, isHandlerError := proxyErr.(caddyhttp.HandlerError) + // if the error occurred after a connection was established, // we have to assume the upstream received the request, and // retries need to be carefully decided, because some requests // are not idempotent - if lb.RetryMatch == nil && req.Method != "GET" { - // by default, don't retry requests if they aren't GET - return false - } - if !lb.RetryMatch.AnyMatch(req) { - return false + if !isDialError && !(isHandlerError && errors.Is(herr, noUpstreamsAvailable)) { + if lb.RetryMatch == nil && req.Method != "GET" { + // by default, don't retry requests if they aren't GET + return false + } + + if !lb.RetryMatch.AnyMatch(req) { + return false + } } } @@ -1421,6 +1427,8 @@ func (c ignoreClientGoneContext) Err() error { // from the proxy handler. const proxyHandleResponseContextCtxKey caddy.CtxKey = "reverse_proxy_handle_response_context" +var noUpstreamsAvailable = fmt.Errorf("no upstreams available") + // Interface guards var ( _ caddy.Provisioner = (*Handler)(nil) -- cgit v1.2.3