From f1859e1693b5664554d310f5d17fdb5104b84626 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Fri, 29 Sep 2023 15:46:22 -0700
Subject: [PATCH] fix: in errorHandler make sure to return the correct error

error handler currently does not return and has never returned
the error back to client application for any in-flight requests
that might have failed or canceled.

make sure we return errors back to client for even transient
in-flight request errors.
---
 main.go | 65 +++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/main.go b/main.go
index cbdeb12..ebdd3df 100644
--- a/main.go
+++ b/main.go
@@ -195,34 +195,49 @@ type BackendStats struct {
 	DowntimeStart   time.Time
 }
 
+const errMessage = `<?xml version="1.0" encoding="UTF-8"?><Error><Code>BackendDown</Code><Message>The remote server returned an error (%v)</Message><Resource>%s</Resource></Error>`
+
+func writeErrorResponse(w http.ResponseWriter, r *http.Request, err error) {
+	// Set retry-after header to indicate user-agents to retry request after 120secs.
+	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
+	w.Header().Set("Retry-After", "120")
+	w.WriteHeader(http.StatusBadGateway)
+	w.Header().Set("Content-Type", "application/xml")
+	fmt.Fprintf(w, errMessage, err, r.URL.Path)
+}
+
 // ErrorHandler called by httputil.ReverseProxy for errors.
 // Avoid canceled context error since it means the client disconnected.
-func (b *Backend) ErrorHandler(_ http.ResponseWriter, _ *http.Request, err error) {
-	if err != nil {
-		offline := true
-		for _, nerr := range []error{
-			context.Canceled,
-			io.EOF,
-			io.ErrClosedPipe,
-			io.ErrUnexpectedEOF,
-			errors.New("http: server closed idle connection"),
-		} {
-			if errors.Is(err, nerr) {
-				offline = false
-				break
-			}
-			if err.Error() == nerr.Error() {
-				offline = false
-				break
-			}
-		}
-		if offline {
-			if globalLoggingEnabled {
-				logMsg(logMessage{Endpoint: b.endpoint, Status: "down", Error: err})
-			}
-			b.setOffline()
+func (b *Backend) ErrorHandler(w http.ResponseWriter, r *http.Request, err error) {
+	if err == nil {
+		panic("reverse proxy cannot call error handler without err being set")
+	}
+
+	offline := true
+	for _, nerr := range []error{
+		context.Canceled,
+		io.EOF,
+		io.ErrClosedPipe,
+		io.ErrUnexpectedEOF,
+		errors.New("http: server closed idle connection"),
+	} {
+		if errors.Is(err, nerr) {
+			offline = false
+			break
+		}
+		if err.Error() == nerr.Error() {
+			offline = false
+			break
+		}
+	}
+	if offline {
+		if globalLoggingEnabled {
+			logMsg(logMessage{Endpoint: b.endpoint, Status: "down", Error: err})
 		}
+		b.setOffline()
 	}
+
+	writeErrorResponse(w, r, err)
 }
 
 // registerMetricsRouter - add handler functions for metrics.
@@ -401,7 +416,7 @@ func (m *multisite) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	}
-	w.WriteHeader(http.StatusBadGateway)
+	writeErrorResponse(w, r, errors.New("all backend servers are offline"))
 }
 
 type site struct {