From d52714c9d6e28abbb80aadb3b2a92eb180df1531 Mon Sep 17 00:00:00 2001
From: Owen Rummage <owen@rummage.cc>
Date: Thu, 20 Aug 2026 11:20:19 -0500
Subject: [PATCH] Retry idempotent requests once on stale keep-alive connection
 reuse

httputil.ReverseProxy pools backend connections via sharedTransport but
has no built-in retry when it picks a pooled connection the backend has
just closed - an inherent HTTP/1.1 keep-alive race. Reproduced this
100% of the time against the LMS backend (lms.noroutes.net) right after
its OIDC login callback: the client got a 502 on the very next request,
with zero trace of that request in LMS's own logs. LMS's side showed
"wthttp/async: handleReadBody(): while waiting for disconnect, received
unexpected data, closing" at the same moment - Wt's lightweight built-in
httpd closing idle connections more eagerly than Go's pool expects.

Retry once, only for idempotent bodyless requests (GET/HEAD/OPTIONS),
directly against sharedTransport from within ErrorHandler - standard
practice for reverse proxies (nginx's proxy_next_upstream, Traefik's
retry middleware). Safe because ReverseProxy hasn't written any response
bytes yet when ErrorHandler runs.
---
 internal/proxy/reverseproxy.go | 43 ++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/internal/proxy/reverseproxy.go b/internal/proxy/reverseproxy.go
index d876fdb..c13f415 100644
--- a/internal/proxy/reverseproxy.go
+++ b/internal/proxy/reverseproxy.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"io"
 	"log/slog"
 	"net"
 	"net/http"
@@ -93,6 +94,35 @@ func buildReverseProxy(rt *runtime.ProxyHostRuntime, backend *loadbalancer.Backe
 	}
 
 	errorHandler := func(w http.ResponseWriter, r *http.Request, err error) {
+		// sharedTransport pools keep-alive connections to the backend. There's
+		// an inherent race in any HTTP/1.1 keep-alive client: the backend can
+		// decide to close an idle connection at (almost) the same moment we
+		// pick it back out of the pool to send the next request, so the
+		// request lands on a connection that's already gone. Go's transport
+		// already retries this automatically in the common case, but not
+		// always (e.g. lightweight backends like Wt's built-in wthttpd, which
+		// can close more eagerly than Go expects - visible in LMS's own logs
+		// as "handleReadBody(): while waiting for disconnect, received
+		// unexpected data, closing"). For idempotent requests with no body,
+		// retrying once here is safe and is standard practice for reverse
+		// proxies (nginx's proxy_next_upstream, Traefik's retry middleware).
+		if isRetryable(r) {
+			log.Warn("backend request failed, retrying once", "proxy_host", rt.Model.Name, "backend", targetAddr, "err", err)
+			retryReq := r.Clone(r.Context())
+			if resp, rerr := sharedTransport.RoundTrip(retryReq); rerr == nil {
+				defer resp.Body.Close()
+				for k, vv := range resp.Header {
+					for _, v := range vv {
+						w.Header().Add(k, v)
+					}
+				}
+				applyCustomHeaders(w.Header(), rt.Model.CustomHeaders, models.HeaderDirectionResponse)
+				w.WriteHeader(resp.StatusCode)
+				io.Copy(w, resp.Body)
+				return
+			}
+		}
+
 		log.Warn("backend request failed", "proxy_host", rt.Model.Name, "backend", targetAddr, "err", err)
 		metrics.HTTPRequestsTotal.WithLabelValues(rt.Model.Name, r.Method, "502").Inc()
 		http.Error(w, "Bad Gateway", http.StatusBadGateway)
@@ -107,6 +137,19 @@ func buildReverseProxy(rt *runtime.ProxyHostRuntime, backend *loadbalancer.Backe
 	}
 }
 
+// isRetryable reports whether it's safe to silently retry a failed backend
+// round trip. Only idempotent, bodyless requests qualify: retrying anything
+// with a request body risks double-submitting it (the body reader may
+// already be partially or fully consumed by the failed attempt).
+func isRetryable(r *http.Request) bool {
+	switch r.Method {
+	case http.MethodGet, http.MethodHead, http.MethodOptions:
+	default:
+		return false
+	}
+	return r.Body == nil || r.Body == http.NoBody || r.ContentLength == 0
+}
+
 func applyCustomHeaders(h http.Header, headers []models.CustomHeader, direction models.CustomHeaderDirection) {
 	for _, c := range headers {
 		if c.Direction != direction {