From adb19010f4417bce9d070f2572a5f02f50b12808 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Tue, 7 Mar 2023 18:45:22 +0000
Subject: [PATCH 01/19] Draft proposal for supporting retries of failed proxy
 requests when backend instances are unavailable

---
 middleware/proxy.go | 135 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 108 insertions(+), 27 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 74f49de8a..def233915 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -29,6 +29,17 @@ type (
 		// Required.
 		Balancer ProxyBalancer
 
+		// RetryProvider defines a provider for handling proxy requests that fail due to a ProxyTarget
+		// being unavailable. The default value is DefaultProxyRetryProvider, which will never retry
+		// failed requests.
+		//
+		// The Proxy middleware will use the configured provider to get a ProxyRetryHandler for
+		// each request. When a request to a target backend fails due to the backend being unavailable,
+		// the middleware will retry the request using the next available ProxyTarget if the provided
+		// ProxyRetryHandler returns true. If the handler returns false, the error will be returned to
+		// the client.
+		RetryProvider ProxyRetryProvider
+
 		// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
 		// retrieved by index e.g. $1, $2 and so on.
 		// Examples:
@@ -76,6 +87,25 @@ type (
 		NextTarget(echo.Context) (*ProxyTarget, error)
 	}
 
+	// ProxyRetryProvider defines an interface that provides a ProxyRetryHandler.
+	ProxyRetryProvider interface {
+		RetryHandler() ProxyRetryHandler
+	}
+
+	// ProxyRetryHandler defines a function that determines if a failed request to an unavailable ProxyTarget should
+	// be retried using the next available ProxyTarget. When the function returns true, the request will be retried.
+	ProxyRetryHandler func(c echo.Context, e error) bool
+
+	// noneProxyRetryProvider implements a ProxyRetryProvider that never retries requests
+	noneProxyRetryProvider struct {
+	}
+
+	// reattemptProxyRetryProvider implements a ProxyRetryProvider that reattempts requests to unavailable
+	// backends a fixed number of times
+	reattemptProxyRetryProvider struct {
+		retries int
+	}
+
 	commonBalancer struct {
 		targets []*ProxyTarget
 		mutex   sync.Mutex
@@ -96,10 +126,14 @@ type (
 )
 
 var (
+	// DefaultProxyRetryProvider is the default ProxyRetryProvider
+	DefaultProxyRetryProvider = &noneProxyRetryProvider{}
+
 	// DefaultProxyConfig is the default Proxy middleware config.
 	DefaultProxyConfig = ProxyConfig{
-		Skipper:    DefaultSkipper,
-		ContextKey: "target",
+		Skipper:       DefaultSkipper,
+		RetryProvider: DefaultProxyRetryProvider,
+		ContextKey:    "target",
 	}
 )
 
@@ -141,6 +175,26 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 	})
 }
 
+func (r *noneProxyRetryProvider) RetryHandler() ProxyRetryHandler {
+	return func(c echo.Context, e error) bool {
+		return false
+	}
+}
+
+func (r *reattemptProxyRetryProvider) RetryHandler() ProxyRetryHandler {
+	attempts := r.retries
+	return func(c echo.Context, e error) bool {
+		attempts--
+		return attempts > 0
+	}
+}
+
+func NewReattepmtProxyRetryProvider(retries int) ProxyRetryProvider {
+	r := reattemptProxyRetryProvider{}
+	r.retries = retries
+	return &r
+}
+
 // NewRandomBalancer returns a random proxy balancer.
 func NewRandomBalancer(targets []*ProxyTarget) ProxyBalancer {
 	b := randomBalancer{}
@@ -236,6 +290,9 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 	if config.Skipper == nil {
 		config.Skipper = DefaultProxyConfig.Skipper
 	}
+	if config.RetryProvider == nil {
+		config.RetryProvider = DefaultProxyConfig.RetryProvider
+	}
 	if config.Balancer == nil {
 		panic("echo: proxy middleware requires balancer")
 	}
@@ -251,25 +308,13 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 
 	provider, isTargetProvider := config.Balancer.(TargetProvider)
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
-		return func(c echo.Context) (err error) {
+		return func(c echo.Context) error {
 			if config.Skipper(c) {
 				return next(c)
 			}
 
 			req := c.Request()
 			res := c.Response()
-
-			var tgt *ProxyTarget
-			if isTargetProvider {
-				tgt, err = provider.NextTarget(c)
-				if err != nil {
-					return err
-				}
-			} else {
-				tgt = config.Balancer.Next(c)
-			}
-			c.Set(config.ContextKey, tgt)
-
 			if err := rewriteURL(config.RegexRewrite, req); err != nil {
 				return err
 			}
@@ -287,19 +332,55 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 				req.Header.Set(echo.HeaderXForwardedFor, c.RealIP())
 			}
 
-			// Proxy
-			switch {
-			case c.IsWebSocket():
-				proxyRaw(tgt, c).ServeHTTP(res, req)
-			case req.Header.Get(echo.HeaderAccept) == "text/event-stream":
-			default:
-				proxyHTTP(tgt, c, config).ServeHTTP(res, req)
-			}
-			if e, ok := c.Get("_error").(error); ok {
-				err = e
-			}
+			retryHandler := config.RetryProvider.RetryHandler()
+			for {
+				var tgt *ProxyTarget
+				var err error
+				if isTargetProvider {
+					tgt, err = provider.NextTarget(c)
+					if err != nil {
+						return err
+					}
+				} else {
+					tgt = config.Balancer.Next(c)
+				}
+				c.Set(config.ContextKey, tgt)
+
+				// Proxy
+				switch {
+				case c.IsWebSocket():
+					proxyRaw(tgt, c).ServeHTTP(res, req)
+				case req.Header.Get(echo.HeaderAccept) == "text/event-stream":
+				default:
+					proxyHTTP(tgt, c, config).ServeHTTP(res, req)
+				}
 
-			return
+				//If there was no error, return
+				e, hasError := c.Get("_error").(error)
+				if !hasError {
+					return nil
+				}
+
+				retry := false
+				if httpErr, ok := e.(*echo.HTTPError); ok {
+					//If the error was http.StatusBadGateway, check the retry handler to
+					//determine if this request should be retried with the next available
+					//ProxyTarget
+					if httpErr.Code == http.StatusBadGateway {
+						retry = retryHandler(c, e)
+					}
+				}
+
+				//If we are not retrying this request, return the error
+				if !retry {
+					return e
+				}
+
+				//Otherwise, try again. Clear the previous error and target
+				//server from context.
+				c.Set("_error", nil)
+				c.Set(config.ContextKey, nil)
+			}
 		}
 	}
 }

From 46e5bd5163a7ca01786d7256c1da4b3ea80667cc Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 10 Mar 2023 11:36:37 +0000
Subject: [PATCH 02/19] Add retry count as first class feature, simplify
 callback, add tests

---
 middleware/proxy.go      |  88 ++++++-------------
 middleware/proxy_test.go | 183 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 210 insertions(+), 61 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index def233915..58b95ef09 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -29,16 +29,17 @@ type (
 		// Required.
 		Balancer ProxyBalancer
 
-		// RetryProvider defines a provider for handling proxy requests that fail due to a ProxyTarget
-		// being unavailable. The default value is DefaultProxyRetryProvider, which will never retry
-		// failed requests.
-		//
-		// The Proxy middleware will use the configured provider to get a ProxyRetryHandler for
-		// each request. When a request to a target backend fails due to the backend being unavailable,
-		// the middleware will retry the request using the next available ProxyTarget if the provided
-		// ProxyRetryHandler returns true. If the handler returns false, the error will be returned to
-		// the client.
-		RetryProvider ProxyRetryProvider
+		// RetryCount defines the number of times a proxied request to an unavailable ProxyTarget
+		// should be retried using the next available ProxyTarget. Defaults to 0, meaning requests
+		// are never retried.
+		RetryCount int
+
+		// RetryHandler defines a function used to determine if a failed request to an unavailable ProxyTarget
+		// should be retried. The RetryHandler will only be called when the number of previous retries is less
+		// than RetryCount. If the function returns true, the request will be retried. When not specified,
+		// DefaultProxyRetryHandler will be used, which will always retry requests. A user defined ProxyRetryHandler
+		// can be provided to only retry specific requests, for example only retry GET requests.
+		RetryHandler ProxyRetryHandler
 
 		// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
 		// retrieved by index e.g. $1, $2 and so on.
@@ -87,24 +88,9 @@ type (
 		NextTarget(echo.Context) (*ProxyTarget, error)
 	}
 
-	// ProxyRetryProvider defines an interface that provides a ProxyRetryHandler.
-	ProxyRetryProvider interface {
-		RetryHandler() ProxyRetryHandler
-	}
-
 	// ProxyRetryHandler defines a function that determines if a failed request to an unavailable ProxyTarget should
 	// be retried using the next available ProxyTarget. When the function returns true, the request will be retried.
-	ProxyRetryHandler func(c echo.Context, e error) bool
-
-	// noneProxyRetryProvider implements a ProxyRetryProvider that never retries requests
-	noneProxyRetryProvider struct {
-	}
-
-	// reattemptProxyRetryProvider implements a ProxyRetryProvider that reattempts requests to unavailable
-	// backends a fixed number of times
-	reattemptProxyRetryProvider struct {
-		retries int
-	}
+	ProxyRetryHandler func(c echo.Context) bool
 
 	commonBalancer struct {
 		targets []*ProxyTarget
@@ -126,14 +112,11 @@ type (
 )
 
 var (
-	// DefaultProxyRetryProvider is the default ProxyRetryProvider
-	DefaultProxyRetryProvider = &noneProxyRetryProvider{}
-
 	// DefaultProxyConfig is the default Proxy middleware config.
 	DefaultProxyConfig = ProxyConfig{
-		Skipper:       DefaultSkipper,
-		RetryProvider: DefaultProxyRetryProvider,
-		ContextKey:    "target",
+		Skipper:      DefaultSkipper,
+		ContextKey:   "target",
+		RetryHandler: DefaultProxyRetryHandler,
 	}
 )
 
@@ -175,24 +158,9 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 	})
 }
 
-func (r *noneProxyRetryProvider) RetryHandler() ProxyRetryHandler {
-	return func(c echo.Context, e error) bool {
-		return false
-	}
-}
-
-func (r *reattemptProxyRetryProvider) RetryHandler() ProxyRetryHandler {
-	attempts := r.retries
-	return func(c echo.Context, e error) bool {
-		attempts--
-		return attempts > 0
-	}
-}
-
-func NewReattepmtProxyRetryProvider(retries int) ProxyRetryProvider {
-	r := reattemptProxyRetryProvider{}
-	r.retries = retries
-	return &r
+// DefaultProxyRetryHandler is a ProxyRetryHandler that always retries requests
+func DefaultProxyRetryHandler(_ echo.Context) bool {
+	return true
 }
 
 // NewRandomBalancer returns a random proxy balancer.
@@ -286,17 +254,16 @@ func Proxy(balancer ProxyBalancer) echo.MiddlewareFunc {
 // ProxyWithConfig returns a Proxy middleware with config.
 // See: `Proxy()`
 func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
+	if config.Balancer == nil {
+		panic("echo: proxy middleware requires balancer")
+	}
 	// Defaults
 	if config.Skipper == nil {
 		config.Skipper = DefaultProxyConfig.Skipper
 	}
-	if config.RetryProvider == nil {
-		config.RetryProvider = DefaultProxyConfig.RetryProvider
+	if config.RetryHandler == nil {
+		config.RetryHandler = DefaultProxyConfig.RetryHandler
 	}
-	if config.Balancer == nil {
-		panic("echo: proxy middleware requires balancer")
-	}
-
 	if config.Rewrite != nil {
 		if config.RegexRewrite == nil {
 			config.RegexRewrite = make(map[*regexp.Regexp]string)
@@ -332,7 +299,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 				req.Header.Set(echo.HeaderXForwardedFor, c.RealIP())
 			}
 
-			retryHandler := config.RetryProvider.RetryHandler()
+			retries := config.RetryCount
 			for {
 				var tgt *ProxyTarget
 				var err error
@@ -363,11 +330,8 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 
 				retry := false
 				if httpErr, ok := e.(*echo.HTTPError); ok {
-					//If the error was http.StatusBadGateway, check the retry handler to
-					//determine if this request should be retried with the next available
-					//ProxyTarget
 					if httpErr.Code == http.StatusBadGateway {
-						retry = retryHandler(c, e)
+						retry = retries > 0 && config.RetryHandler(c)
 					}
 				}
 
@@ -376,6 +340,8 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 					return e
 				}
 
+				retries--
+
 				//Otherwise, try again. Clear the previous error and target
 				//server from context.
 				c.Set("_error", nil)
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 122dddeba..78a5c7b4f 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -393,6 +393,189 @@ func TestProxyError(t *testing.T) {
 	assert.Equal(t, http.StatusBadGateway, rec.Code)
 }
 
+func TestProxyRetries(t *testing.T) {
+
+	newServer := func(res int) (*url.URL, *httptest.Server) {
+		server := httptest.NewServer(
+			http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(res)
+			}),
+		)
+		targetUrl, _ := url.Parse(server.URL)
+		return targetUrl, server
+	}
+
+	targetUrl, server := newServer(http.StatusOK)
+	defer server.Close()
+	goodTarget := &ProxyTarget{
+		Name: "Good",
+		URL:  targetUrl,
+	}
+
+	targetUrl, server = newServer(http.StatusBadRequest)
+	defer server.Close()
+	goodTargetWith40X := &ProxyTarget{
+		Name: "Good with 40X",
+		URL:  targetUrl,
+	}
+
+	targetUrl, _ = url.Parse("http://127.0.0.1:27121")
+	badTarget := &ProxyTarget{
+		Name: "Bad",
+		URL:  targetUrl,
+	}
+
+	var doRetryHandler ProxyRetryHandler = func(c echo.Context) bool { return true }
+	var dontRetryHandler ProxyRetryHandler = func(c echo.Context) bool { return false }
+
+	testCases := []struct {
+		name             string
+		retryCount       int
+		retryHandlers    []ProxyRetryHandler
+		targets          []*ProxyTarget
+		expectedResponse int
+	}{
+		{
+			"retry count 0 does not attempt retry on fail",
+			0,
+			nil,
+			[]*ProxyTarget{
+				badTarget,
+				goodTarget,
+			},
+			http.StatusBadGateway,
+		},
+		{
+			"retry count 1 does not attempt retry on success",
+			1,
+			nil,
+			[]*ProxyTarget{
+				goodTarget,
+			},
+			http.StatusOK,
+		},
+		{
+			"retry count 1 does retry on handler return true",
+			1,
+			[]ProxyRetryHandler{
+				doRetryHandler,
+			},
+			[]*ProxyTarget{
+				badTarget,
+				goodTarget,
+			},
+			http.StatusOK,
+		},
+		{
+			"retry count 1 does not retry on handler return false",
+			1,
+			[]ProxyRetryHandler{
+				dontRetryHandler,
+			},
+			[]*ProxyTarget{
+				badTarget,
+				goodTarget,
+			},
+			http.StatusBadGateway,
+		},
+		{
+			"retry count 2 returns error when no more retries left",
+			2,
+			[]ProxyRetryHandler{
+				doRetryHandler,
+				doRetryHandler,
+			},
+			[]*ProxyTarget{
+				badTarget,
+				badTarget,
+				badTarget,
+				goodTarget, //Should never be reached as only 2 retries
+			},
+			http.StatusBadGateway,
+		},
+		{
+			"retry count 2 returns error when retries left by handler returns false",
+			3,
+			[]ProxyRetryHandler{
+				doRetryHandler,
+				doRetryHandler,
+				dontRetryHandler,
+			},
+			[]*ProxyTarget{
+				badTarget,
+				badTarget,
+				badTarget,
+				goodTarget, //Should never be reached as retry handler returns false on 2nd check
+			},
+			http.StatusBadGateway,
+		},
+		{
+			"retry count 3 succeeds",
+			3,
+			[]ProxyRetryHandler{
+				doRetryHandler,
+				doRetryHandler,
+				doRetryHandler,
+			},
+			[]*ProxyTarget{
+				badTarget,
+				badTarget,
+				badTarget,
+				goodTarget,
+			},
+			http.StatusOK,
+		},
+		{
+			"40x responses are not retried",
+			1,
+			nil,
+			[]*ProxyTarget{
+				goodTargetWith40X,
+				goodTarget,
+			},
+			http.StatusBadRequest,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+
+			retryHandlerCall := 0
+			retryHandler := func(c echo.Context) bool {
+				if len(tc.retryHandlers) == 0 {
+					assert.FailNow(t, fmt.Sprintf("unexpected calls, %d, to retry handler", retryHandlerCall))
+				}
+
+				retryHandlerCall++
+
+				nextRetryHandler := tc.retryHandlers[0]
+				tc.retryHandlers = tc.retryHandlers[1:]
+
+				return nextRetryHandler(c)
+			}
+
+			e := echo.New()
+			e.Use(ProxyWithConfig(
+				ProxyConfig{
+					Balancer:     NewRoundRobinBalancer(tc.targets),
+					RetryCount:   tc.retryCount,
+					RetryHandler: retryHandler,
+				},
+			))
+
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			rec := httptest.NewRecorder()
+
+			e.ServeHTTP(rec, req)
+
+			assert.Equal(t, tc.expectedResponse, rec.Code)
+			if len(tc.retryHandlers) > 0 {
+				assert.FailNow(t, fmt.Sprintf("expected %d more retry handler calls", len(tc.retryHandlers)))
+			}
+		})
+	}
+}
+
 func TestClientCancelConnectionResultsHTTPCode499(t *testing.T) {
 	var timeoutStop sync.WaitGroup
 	timeoutStop.Add(1)

From 7c35cda9e94e021a27dbc707a445438a3a459e7c Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 10 Mar 2023 15:08:51 +0000
Subject: [PATCH 03/19] Minor comment tidy up

---
 middleware/proxy.go      | 29 +++++++++++++++--------------
 middleware/proxy_test.go |  2 +-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 58b95ef09..e6594cb5b 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -29,15 +29,16 @@ type (
 		// Required.
 		Balancer ProxyBalancer
 
-		// RetryCount defines the number of times a proxied request to an unavailable ProxyTarget
-		// should be retried using the next available ProxyTarget. Defaults to 0, meaning requests
-		// are never retried.
+		// RetryCount defines the number of times a proxied request to an unavailable
+		// ProxyTarget should be retried using the next available ProxyTarget. Defaults
+		// to 0, meaning requests are never retried.
 		RetryCount int
 
-		// RetryHandler defines a function used to determine if a failed request to an unavailable ProxyTarget
-		// should be retried. The RetryHandler will only be called when the number of previous retries is less
-		// than RetryCount. If the function returns true, the request will be retried. When not specified,
-		// DefaultProxyRetryHandler will be used, which will always retry requests. A user defined ProxyRetryHandler
+		// RetryHandler defines a function used to determine if a failed request to an
+		// unavailable ProxyTarget should be retried. The RetryHandler will only be called
+		// when the number of previous retries is less than RetryCount. If the function returns
+		// true, the request will be retried. When not specified, DefaultProxyRetryHandler
+		// will be used, which will always retry requests. A user defined ProxyRetryHandler
 		// can be provided to only retry specific requests, for example only retry GET requests.
 		RetryHandler ProxyRetryHandler
 
@@ -83,13 +84,15 @@ type (
 		Next(echo.Context) *ProxyTarget
 	}
 
-	// TargetProvider defines an interface that gives the opportunity for balancer to return custom errors when selecting target.
+	// TargetProvider defines an interface that gives the opportunity for balancer
+	// to return custom errors when selecting target.
 	TargetProvider interface {
 		NextTarget(echo.Context) (*ProxyTarget, error)
 	}
 
-	// ProxyRetryHandler defines a function that determines if a failed request to an unavailable ProxyTarget should
-	// be retried using the next available ProxyTarget. When the function returns true, the request will be retried.
+	// ProxyRetryHandler defines a function that determines if a failed request to
+	// an unavailable ProxyTarget should be retried using the next available ProxyTarget.
+	// When the function returns true, the request will be retried.
 	ProxyRetryHandler func(c echo.Context) bool
 
 	commonBalancer struct {
@@ -159,7 +162,7 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 }
 
 // DefaultProxyRetryHandler is a ProxyRetryHandler that always retries requests
-func DefaultProxyRetryHandler(_ echo.Context) bool {
+func DefaultProxyRetryHandler(c echo.Context) bool {
 	return true
 }
 
@@ -322,7 +325,6 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 					proxyHTTP(tgt, c, config).ServeHTTP(res, req)
 				}
 
-				//If there was no error, return
 				e, hasError := c.Get("_error").(error)
 				if !hasError {
 					return nil
@@ -335,14 +337,13 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 					}
 				}
 
-				//If we are not retrying this request, return the error
 				if !retry {
 					return e
 				}
 
 				retries--
 
-				//Otherwise, try again. Clear the previous error and target
+				//Try request again. Clear the previous error and target
 				//server from context.
 				c.Set("_error", nil)
 				c.Set(config.ContextKey, nil)
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 78a5c7b4f..cad409bab 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -494,7 +494,7 @@ func TestProxyRetries(t *testing.T) {
 			http.StatusBadGateway,
 		},
 		{
-			"retry count 2 returns error when retries left by handler returns false",
+			"retry count 2 returns error when retries left but handler returns false",
 			3,
 			[]ProxyRetryHandler{
 				doRetryHandler,

From 678da72a6f4c5f765ddcd112bf4eb93e33678d3e Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 10 Mar 2023 15:22:46 +0000
Subject: [PATCH 04/19] Rename retry handler to filter

---
 middleware/proxy.go      | 30 +++++++++----------
 middleware/proxy_test.go | 62 ++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index e6594cb5b..b27660880 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -34,13 +34,13 @@ type (
 		// to 0, meaning requests are never retried.
 		RetryCount int
 
-		// RetryHandler defines a function used to determine if a failed request to an
-		// unavailable ProxyTarget should be retried. The RetryHandler will only be called
+		// RetryFilter defines a function used to determine if a failed request to an
+		// unavailable ProxyTarget should be retried. The RetryFilter will only be called
 		// when the number of previous retries is less than RetryCount. If the function returns
-		// true, the request will be retried. When not specified, DefaultProxyRetryHandler
-		// will be used, which will always retry requests. A user defined ProxyRetryHandler
+		// true, the request will be retried. When not specified, DefaultProxyRetryFilter
+		// will be used, which will always retry requests. A user defined ProxyRetryFilter
 		// can be provided to only retry specific requests, for example only retry GET requests.
-		RetryHandler ProxyRetryHandler
+		RetryFilter ProxyRetryFilter
 
 		// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
 		// retrieved by index e.g. $1, $2 and so on.
@@ -90,10 +90,10 @@ type (
 		NextTarget(echo.Context) (*ProxyTarget, error)
 	}
 
-	// ProxyRetryHandler defines a function that determines if a failed request to
+	// ProxyRetryFilter defines a function that determines if a failed request to
 	// an unavailable ProxyTarget should be retried using the next available ProxyTarget.
 	// When the function returns true, the request will be retried.
-	ProxyRetryHandler func(c echo.Context) bool
+	ProxyRetryFilter func(c echo.Context) bool
 
 	commonBalancer struct {
 		targets []*ProxyTarget
@@ -117,9 +117,9 @@ type (
 var (
 	// DefaultProxyConfig is the default Proxy middleware config.
 	DefaultProxyConfig = ProxyConfig{
-		Skipper:      DefaultSkipper,
-		ContextKey:   "target",
-		RetryHandler: DefaultProxyRetryHandler,
+		Skipper:     DefaultSkipper,
+		ContextKey:  "target",
+		RetryFilter: DefaultProxyRetryFilter,
 	}
 )
 
@@ -161,8 +161,8 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 	})
 }
 
-// DefaultProxyRetryHandler is a ProxyRetryHandler that always retries requests
-func DefaultProxyRetryHandler(c echo.Context) bool {
+// DefaultProxyRetryFilter is a ProxyRetryFilter that always retries requests
+func DefaultProxyRetryFilter(c echo.Context) bool {
 	return true
 }
 
@@ -264,8 +264,8 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 	if config.Skipper == nil {
 		config.Skipper = DefaultProxyConfig.Skipper
 	}
-	if config.RetryHandler == nil {
-		config.RetryHandler = DefaultProxyConfig.RetryHandler
+	if config.RetryFilter == nil {
+		config.RetryFilter = DefaultProxyConfig.RetryFilter
 	}
 	if config.Rewrite != nil {
 		if config.RegexRewrite == nil {
@@ -333,7 +333,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 				retry := false
 				if httpErr, ok := e.(*echo.HTTPError); ok {
 					if httpErr.Code == http.StatusBadGateway {
-						retry = retries > 0 && config.RetryHandler(c)
+						retry = retries > 0 && config.RetryFilter(c)
 					}
 				}
 
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index cad409bab..ca10dfa8c 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -425,13 +425,13 @@ func TestProxyRetries(t *testing.T) {
 		URL:  targetUrl,
 	}
 
-	var doRetryHandler ProxyRetryHandler = func(c echo.Context) bool { return true }
-	var dontRetryHandler ProxyRetryHandler = func(c echo.Context) bool { return false }
+	var alwaysRetryFilter ProxyRetryFilter = func(c echo.Context) bool { return true }
+	var neverRetryFilter ProxyRetryFilter = func(c echo.Context) bool { return false }
 
 	testCases := []struct {
 		name             string
 		retryCount       int
-		retryHandlers    []ProxyRetryHandler
+		retryFilters     []ProxyRetryFilter
 		targets          []*ProxyTarget
 		expectedResponse int
 	}{
@@ -457,8 +457,8 @@ func TestProxyRetries(t *testing.T) {
 		{
 			"retry count 1 does retry on handler return true",
 			1,
-			[]ProxyRetryHandler{
-				doRetryHandler,
+			[]ProxyRetryFilter{
+				alwaysRetryFilter,
 			},
 			[]*ProxyTarget{
 				badTarget,
@@ -469,8 +469,8 @@ func TestProxyRetries(t *testing.T) {
 		{
 			"retry count 1 does not retry on handler return false",
 			1,
-			[]ProxyRetryHandler{
-				dontRetryHandler,
+			[]ProxyRetryFilter{
+				neverRetryFilter,
 			},
 			[]*ProxyTarget{
 				badTarget,
@@ -481,9 +481,9 @@ func TestProxyRetries(t *testing.T) {
 		{
 			"retry count 2 returns error when no more retries left",
 			2,
-			[]ProxyRetryHandler{
-				doRetryHandler,
-				doRetryHandler,
+			[]ProxyRetryFilter{
+				alwaysRetryFilter,
+				alwaysRetryFilter,
 			},
 			[]*ProxyTarget{
 				badTarget,
@@ -496,10 +496,10 @@ func TestProxyRetries(t *testing.T) {
 		{
 			"retry count 2 returns error when retries left but handler returns false",
 			3,
-			[]ProxyRetryHandler{
-				doRetryHandler,
-				doRetryHandler,
-				dontRetryHandler,
+			[]ProxyRetryFilter{
+				alwaysRetryFilter,
+				alwaysRetryFilter,
+				neverRetryFilter,
 			},
 			[]*ProxyTarget{
 				badTarget,
@@ -512,10 +512,10 @@ func TestProxyRetries(t *testing.T) {
 		{
 			"retry count 3 succeeds",
 			3,
-			[]ProxyRetryHandler{
-				doRetryHandler,
-				doRetryHandler,
-				doRetryHandler,
+			[]ProxyRetryFilter{
+				alwaysRetryFilter,
+				alwaysRetryFilter,
+				alwaysRetryFilter,
 			},
 			[]*ProxyTarget{
 				badTarget,
@@ -540,26 +540,26 @@ func TestProxyRetries(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 
-			retryHandlerCall := 0
-			retryHandler := func(c echo.Context) bool {
-				if len(tc.retryHandlers) == 0 {
-					assert.FailNow(t, fmt.Sprintf("unexpected calls, %d, to retry handler", retryHandlerCall))
+			retryFilterCall := 0
+			retryFilter := func(c echo.Context) bool {
+				if len(tc.retryFilters) == 0 {
+					assert.FailNow(t, fmt.Sprintf("unexpected calls, %d, to retry handler", retryFilterCall))
 				}
 
-				retryHandlerCall++
+				retryFilterCall++
 
-				nextRetryHandler := tc.retryHandlers[0]
-				tc.retryHandlers = tc.retryHandlers[1:]
+				nextRetryFilter := tc.retryFilters[0]
+				tc.retryFilters = tc.retryFilters[1:]
 
-				return nextRetryHandler(c)
+				return nextRetryFilter(c)
 			}
 
 			e := echo.New()
 			e.Use(ProxyWithConfig(
 				ProxyConfig{
-					Balancer:     NewRoundRobinBalancer(tc.targets),
-					RetryCount:   tc.retryCount,
-					RetryHandler: retryHandler,
+					Balancer:    NewRoundRobinBalancer(tc.targets),
+					RetryCount:  tc.retryCount,
+					RetryFilter: retryFilter,
 				},
 			))
 
@@ -569,8 +569,8 @@ func TestProxyRetries(t *testing.T) {
 			e.ServeHTTP(rec, req)
 
 			assert.Equal(t, tc.expectedResponse, rec.Code)
-			if len(tc.retryHandlers) > 0 {
-				assert.FailNow(t, fmt.Sprintf("expected %d more retry handler calls", len(tc.retryHandlers)))
+			if len(tc.retryFilters) > 0 {
+				assert.FailNow(t, fmt.Sprintf("expected %d more retry handler calls", len(tc.retryFilters)))
 			}
 		})
 	}

From 675c19eded3d26ffc0f7d2ceb9322162b5b69e0c Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Sat, 11 Mar 2023 09:39:59 +0000
Subject: [PATCH 05/19] use named fields for structs

---
 middleware/proxy_test.go | 76 +++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 40 deletions(-)

diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index ca10dfa8c..0da1a82ec 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -436,104 +436,100 @@ func TestProxyRetries(t *testing.T) {
 		expectedResponse int
 	}{
 		{
-			"retry count 0 does not attempt retry on fail",
-			0,
-			nil,
-			[]*ProxyTarget{
+			name: "retry count 0 does not attempt retry on fail",
+			targets: []*ProxyTarget{
 				badTarget,
 				goodTarget,
 			},
-			http.StatusBadGateway,
+			expectedResponse: http.StatusBadGateway,
 		},
 		{
-			"retry count 1 does not attempt retry on success",
-			1,
-			nil,
-			[]*ProxyTarget{
+			name:       "retry count 1 does not attempt retry on success",
+			retryCount: 1,
+			targets: []*ProxyTarget{
 				goodTarget,
 			},
-			http.StatusOK,
+			expectedResponse: http.StatusOK,
 		},
 		{
-			"retry count 1 does retry on handler return true",
-			1,
-			[]ProxyRetryFilter{
+			name:       "retry count 1 does retry on handler return true",
+			retryCount: 1,
+			retryFilters: []ProxyRetryFilter{
 				alwaysRetryFilter,
 			},
-			[]*ProxyTarget{
+			targets: []*ProxyTarget{
 				badTarget,
 				goodTarget,
 			},
-			http.StatusOK,
+			expectedResponse: http.StatusOK,
 		},
 		{
-			"retry count 1 does not retry on handler return false",
-			1,
-			[]ProxyRetryFilter{
+			name:       "retry count 1 does not retry on handler return false",
+			retryCount: 1,
+			retryFilters: []ProxyRetryFilter{
 				neverRetryFilter,
 			},
-			[]*ProxyTarget{
+			targets: []*ProxyTarget{
 				badTarget,
 				goodTarget,
 			},
-			http.StatusBadGateway,
+			expectedResponse: http.StatusBadGateway,
 		},
 		{
-			"retry count 2 returns error when no more retries left",
-			2,
-			[]ProxyRetryFilter{
+			name:       "retry count 2 returns error when no more retries left",
+			retryCount: 2,
+			retryFilters: []ProxyRetryFilter{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 			},
-			[]*ProxyTarget{
+			targets: []*ProxyTarget{
 				badTarget,
 				badTarget,
 				badTarget,
 				goodTarget, //Should never be reached as only 2 retries
 			},
-			http.StatusBadGateway,
+			expectedResponse: http.StatusBadGateway,
 		},
 		{
-			"retry count 2 returns error when retries left but handler returns false",
-			3,
-			[]ProxyRetryFilter{
+			name:       "retry count 2 returns error when retries left but handler returns false",
+			retryCount: 3,
+			retryFilters: []ProxyRetryFilter{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 				neverRetryFilter,
 			},
-			[]*ProxyTarget{
+			targets: []*ProxyTarget{
 				badTarget,
 				badTarget,
 				badTarget,
 				goodTarget, //Should never be reached as retry handler returns false on 2nd check
 			},
-			http.StatusBadGateway,
+			expectedResponse: http.StatusBadGateway,
 		},
 		{
-			"retry count 3 succeeds",
-			3,
-			[]ProxyRetryFilter{
+			name:       "retry count 3 succeeds",
+			retryCount: 3,
+			retryFilters: []ProxyRetryFilter{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 			},
-			[]*ProxyTarget{
+			targets: []*ProxyTarget{
 				badTarget,
 				badTarget,
 				badTarget,
 				goodTarget,
 			},
-			http.StatusOK,
+			expectedResponse: http.StatusOK,
 		},
 		{
-			"40x responses are not retried",
-			1,
-			nil,
-			[]*ProxyTarget{
+			name:       "40x responses are not retried",
+			retryCount: 1,
+			targets: []*ProxyTarget{
 				goodTargetWith40X,
 				goodTarget,
 			},
-			http.StatusBadRequest,
+			expectedResponse: http.StatusBadRequest,
 		},
 	}
 

From 5989375345df9f2cb43bd1c78c09e0b9567c2566 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Sat, 11 Mar 2023 09:53:17 +0000
Subject: [PATCH 06/19] Remove function type for filter, don't use Default var
 for default

---
 middleware/proxy.go      | 29 ++++++++++-------------------
 middleware/proxy_test.go | 16 ++++++++--------
 2 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index b27660880..8344e634b 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -36,11 +36,11 @@ type (
 
 		// RetryFilter defines a function used to determine if a failed request to an
 		// unavailable ProxyTarget should be retried. The RetryFilter will only be called
-		// when the number of previous retries is less than RetryCount. If the function returns
-		// true, the request will be retried. When not specified, DefaultProxyRetryFilter
-		// will be used, which will always retry requests. A user defined ProxyRetryFilter
-		// can be provided to only retry specific requests, for example only retry GET requests.
-		RetryFilter ProxyRetryFilter
+		// when the number of previous retries is less than RetryCount. If the function
+		// returns true, the request will be retried. When not specified, all fail requests
+		// will be retried. A user custom RetryFilter can be provided to only retry specific
+		// requests, for example only retry GET requests.
+		RetryFilter func(c echo.Context) bool
 
 		// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
 		// retrieved by index e.g. $1, $2 and so on.
@@ -90,11 +90,6 @@ type (
 		NextTarget(echo.Context) (*ProxyTarget, error)
 	}
 
-	// ProxyRetryFilter defines a function that determines if a failed request to
-	// an unavailable ProxyTarget should be retried using the next available ProxyTarget.
-	// When the function returns true, the request will be retried.
-	ProxyRetryFilter func(c echo.Context) bool
-
 	commonBalancer struct {
 		targets []*ProxyTarget
 		mutex   sync.Mutex
@@ -117,9 +112,8 @@ type (
 var (
 	// DefaultProxyConfig is the default Proxy middleware config.
 	DefaultProxyConfig = ProxyConfig{
-		Skipper:     DefaultSkipper,
-		ContextKey:  "target",
-		RetryFilter: DefaultProxyRetryFilter,
+		Skipper:    DefaultSkipper,
+		ContextKey: "target",
 	}
 )
 
@@ -161,11 +155,6 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 	})
 }
 
-// DefaultProxyRetryFilter is a ProxyRetryFilter that always retries requests
-func DefaultProxyRetryFilter(c echo.Context) bool {
-	return true
-}
-
 // NewRandomBalancer returns a random proxy balancer.
 func NewRandomBalancer(targets []*ProxyTarget) ProxyBalancer {
 	b := randomBalancer{}
@@ -265,7 +254,9 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 		config.Skipper = DefaultProxyConfig.Skipper
 	}
 	if config.RetryFilter == nil {
-		config.RetryFilter = DefaultProxyConfig.RetryFilter
+		config.RetryFilter = func(c echo.Context) bool {
+			return true
+		}
 	}
 	if config.Rewrite != nil {
 		if config.RegexRewrite == nil {
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 0da1a82ec..c6ed74fa1 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -425,13 +425,13 @@ func TestProxyRetries(t *testing.T) {
 		URL:  targetUrl,
 	}
 
-	var alwaysRetryFilter ProxyRetryFilter = func(c echo.Context) bool { return true }
-	var neverRetryFilter ProxyRetryFilter = func(c echo.Context) bool { return false }
+	alwaysRetryFilter := func(c echo.Context) bool { return true }
+	neverRetryFilter := func(c echo.Context) bool { return false }
 
 	testCases := []struct {
 		name             string
 		retryCount       int
-		retryFilters     []ProxyRetryFilter
+		retryFilters     []func(c echo.Context) bool
 		targets          []*ProxyTarget
 		expectedResponse int
 	}{
@@ -454,7 +454,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 1 does retry on handler return true",
 			retryCount: 1,
-			retryFilters: []ProxyRetryFilter{
+			retryFilters: []func(c echo.Context) bool{
 				alwaysRetryFilter,
 			},
 			targets: []*ProxyTarget{
@@ -466,7 +466,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 1 does not retry on handler return false",
 			retryCount: 1,
-			retryFilters: []ProxyRetryFilter{
+			retryFilters: []func(c echo.Context) bool{
 				neverRetryFilter,
 			},
 			targets: []*ProxyTarget{
@@ -478,7 +478,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 2 returns error when no more retries left",
 			retryCount: 2,
-			retryFilters: []ProxyRetryFilter{
+			retryFilters: []func(c echo.Context) bool{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 			},
@@ -493,7 +493,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 2 returns error when retries left but handler returns false",
 			retryCount: 3,
-			retryFilters: []ProxyRetryFilter{
+			retryFilters: []func(c echo.Context) bool{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 				neverRetryFilter,
@@ -509,7 +509,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 3 succeeds",
 			retryCount: 3,
-			retryFilters: []ProxyRetryFilter{
+			retryFilters: []func(c echo.Context) bool{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 				alwaysRetryFilter,

From c12daa707a60e0dcef163853d6586b9a62e4b692 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 16:18:36 +0000
Subject: [PATCH 07/19] Add error to RetryFilter, move BadGateway check to
 default implementation

---
 middleware/proxy.go      | 26 +++++++++++++-------------
 middleware/proxy_test.go | 20 ++++++++++----------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 8344e634b..ece2e768a 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -37,10 +37,13 @@ type (
 		// RetryFilter defines a function used to determine if a failed request to an
 		// unavailable ProxyTarget should be retried. The RetryFilter will only be called
 		// when the number of previous retries is less than RetryCount. If the function
-		// returns true, the request will be retried. When not specified, all fail requests
-		// will be retried. A user custom RetryFilter can be provided to only retry specific
-		// requests, for example only retry GET requests.
-		RetryFilter func(c echo.Context) bool
+		// returns true, the request will be retried. When not specified, all requests that
+		// fail with http.StatusBadGateway error will be retried. A custom RetryFilter can
+		// be provided to only retry specific requests. Note that RetryFilter is only
+		// called when the request to the target fails, or an internal error in the Proxy
+		// middleware has occurred. Successful requests that return a non-200 response code
+		// cannot be retried.
+		RetryFilter func(c echo.Context, e error) bool
 
 		// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
 		// retrieved by index e.g. $1, $2 and so on.
@@ -254,8 +257,11 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 		config.Skipper = DefaultProxyConfig.Skipper
 	}
 	if config.RetryFilter == nil {
-		config.RetryFilter = func(c echo.Context) bool {
-			return true
+		config.RetryFilter = func(c echo.Context, e error) bool {
+			if httpErr, ok := e.(*echo.HTTPError); ok {
+				return httpErr.Code == http.StatusBadGateway
+			}
+			return false
 		}
 	}
 	if config.Rewrite != nil {
@@ -321,13 +327,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 					return nil
 				}
 
-				retry := false
-				if httpErr, ok := e.(*echo.HTTPError); ok {
-					if httpErr.Code == http.StatusBadGateway {
-						retry = retries > 0 && config.RetryFilter(c)
-					}
-				}
-
+				retry := retries > 0 && config.RetryFilter(c, e)
 				if !retry {
 					return e
 				}
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index c6ed74fa1..8bacf6c1d 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -425,13 +425,13 @@ func TestProxyRetries(t *testing.T) {
 		URL:  targetUrl,
 	}
 
-	alwaysRetryFilter := func(c echo.Context) bool { return true }
-	neverRetryFilter := func(c echo.Context) bool { return false }
+	alwaysRetryFilter := func(c echo.Context, e error) bool { return true }
+	neverRetryFilter := func(c echo.Context, e error) bool { return false }
 
 	testCases := []struct {
 		name             string
 		retryCount       int
-		retryFilters     []func(c echo.Context) bool
+		retryFilters     []func(c echo.Context, e error) bool
 		targets          []*ProxyTarget
 		expectedResponse int
 	}{
@@ -454,7 +454,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 1 does retry on handler return true",
 			retryCount: 1,
-			retryFilters: []func(c echo.Context) bool{
+			retryFilters: []func(c echo.Context, e error) bool{
 				alwaysRetryFilter,
 			},
 			targets: []*ProxyTarget{
@@ -466,7 +466,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 1 does not retry on handler return false",
 			retryCount: 1,
-			retryFilters: []func(c echo.Context) bool{
+			retryFilters: []func(c echo.Context, e error) bool{
 				neverRetryFilter,
 			},
 			targets: []*ProxyTarget{
@@ -478,7 +478,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 2 returns error when no more retries left",
 			retryCount: 2,
-			retryFilters: []func(c echo.Context) bool{
+			retryFilters: []func(c echo.Context, e error) bool{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 			},
@@ -493,7 +493,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 2 returns error when retries left but handler returns false",
 			retryCount: 3,
-			retryFilters: []func(c echo.Context) bool{
+			retryFilters: []func(c echo.Context, e error) bool{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 				neverRetryFilter,
@@ -509,7 +509,7 @@ func TestProxyRetries(t *testing.T) {
 		{
 			name:       "retry count 3 succeeds",
 			retryCount: 3,
-			retryFilters: []func(c echo.Context) bool{
+			retryFilters: []func(c echo.Context, e error) bool{
 				alwaysRetryFilter,
 				alwaysRetryFilter,
 				alwaysRetryFilter,
@@ -537,7 +537,7 @@ func TestProxyRetries(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 
 			retryFilterCall := 0
-			retryFilter := func(c echo.Context) bool {
+			retryFilter := func(c echo.Context, e error) bool {
 				if len(tc.retryFilters) == 0 {
 					assert.FailNow(t, fmt.Sprintf("unexpected calls, %d, to retry handler", retryFilterCall))
 				}
@@ -547,7 +547,7 @@ func TestProxyRetries(t *testing.T) {
 				nextRetryFilter := tc.retryFilters[0]
 				tc.retryFilters = tc.retryFilters[1:]
 
-				return nextRetryFilter(c)
+				return nextRetryFilter(c, e)
 			}
 
 			e := echo.New()

From 7299721ef920c7a358938900fca044405228e67a Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 16:37:10 +0000
Subject: [PATCH 08/19] Add ErrorHandler to Proxy middleware

---
 middleware/proxy.go      | 25 +++++++++---
 middleware/proxy_test.go | 83 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 5 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index ece2e768a..85f2f41c4 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -45,6 +45,15 @@ type (
 		// cannot be retried.
 		RetryFilter func(c echo.Context, e error) bool
 
+		// ErrorHandler defines a function which can be used to return custom errors from
+		// the Proxy middleware. ErrorHandler is only invoked when there has been
+		// either an internal error in the Proxy middleware or the ProxyTarget is
+		// unavailable. Due to the way requests are proxied, ErrorHandler is not invoked
+		// when a Proxy target returns a non-200 response. In these cases, the response
+		// is already written so errors cannot be modified. ErrorHandler is only
+		// invoked after all retry attempts have been exhausted.
+		ErrorHandler func(c echo.Context, err error) error
+
 		// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
 		// retrieved by index e.g. $1, $2 and so on.
 		// Examples:
@@ -264,6 +273,11 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 			return false
 		}
 	}
+	if config.ErrorHandler == nil {
+		config.ErrorHandler = func(c echo.Context, err error) error {
+			return err
+		}
+	}
 	if config.Rewrite != nil {
 		if config.RegexRewrite == nil {
 			config.RegexRewrite = make(map[*regexp.Regexp]string)
@@ -274,6 +288,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 	}
 
 	provider, isTargetProvider := config.Balancer.(TargetProvider)
+
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
 		return func(c echo.Context) error {
 			if config.Skipper(c) {
@@ -283,7 +298,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 			req := c.Request()
 			res := c.Response()
 			if err := rewriteURL(config.RegexRewrite, req); err != nil {
-				return err
+				return config.ErrorHandler(c, err)
 			}
 
 			// Fix header
@@ -306,7 +321,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 				if isTargetProvider {
 					tgt, err = provider.NextTarget(c)
 					if err != nil {
-						return err
+						return config.ErrorHandler(c, err)
 					}
 				} else {
 					tgt = config.Balancer.Next(c)
@@ -322,14 +337,14 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 					proxyHTTP(tgt, c, config).ServeHTTP(res, req)
 				}
 
-				e, hasError := c.Get("_error").(error)
+				err, hasError := c.Get("_error").(error)
 				if !hasError {
 					return nil
 				}
 
-				retry := retries > 0 && config.RetryFilter(c, e)
+				retry := retries > 0 && config.RetryFilter(c, err)
 				if !retry {
-					return e
+					return config.ErrorHandler(c, err)
 				}
 
 				retries--
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 8bacf6c1d..5b53b76b6 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -3,6 +3,7 @@ package middleware
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -572,6 +573,88 @@ func TestProxyRetries(t *testing.T) {
 	}
 }
 
+func TestProxyErrorHandler(t *testing.T) {
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	goodUrl, _ := url.Parse(server.URL)
+	defer server.Close()
+	goodTarget := &ProxyTarget{
+		Name: "Good",
+		URL:  goodUrl,
+	}
+
+	badUrl, _ := url.Parse("http://127.0.0.1:27121")
+	badTarget := &ProxyTarget{
+		Name: "Bad",
+		URL:  badUrl,
+	}
+
+	transformedError := errors.New("a new error")
+	//Not called when no error
+	//Called when error and returned error passed to central
+	//
+	testCases := []struct {
+		name             string
+		target           *ProxyTarget
+		errorHandler     func(c echo.Context, e error) error
+		expectFinalError func(t *testing.T, err error)
+	}{
+		{
+			name:   "Error handler not invoked when request success",
+			target: goodTarget,
+			errorHandler: func(c echo.Context, e error) error {
+				assert.FailNow(t, "error handler should not be invoked")
+				return e
+			},
+		},
+		{
+			name:   "Error handler invoked when request fails",
+			target: badTarget,
+			errorHandler: func(c echo.Context, e error) error {
+				httpErr, ok := e.(*echo.HTTPError)
+				assert.True(t, ok, "expected http error to be passed to handler")
+				assert.Equal(t, http.StatusBadGateway, httpErr.Code, "expected http bad gateway error to be passed to handler")
+				return transformedError
+			},
+			expectFinalError: func(t *testing.T, err error) {
+				assert.Equal(t, transformedError, err, "transformed error not returned from proxy")
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := echo.New()
+			e.Use(ProxyWithConfig(
+				ProxyConfig{
+					Balancer:     NewRoundRobinBalancer([]*ProxyTarget{tc.target}),
+					ErrorHandler: tc.errorHandler,
+				},
+			))
+
+			errorHandlerCalled := false
+			e.HTTPErrorHandler = func(err error, c echo.Context) {
+				errorHandlerCalled = true
+				tc.expectFinalError(t, err)
+				e.DefaultHTTPErrorHandler(err, c)
+			}
+
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			rec := httptest.NewRecorder()
+
+			e.ServeHTTP(rec, req)
+
+			if !errorHandlerCalled && tc.expectFinalError != nil {
+				t.Fatalf("error handler was not called")
+			}
+
+		})
+	}
+}
+
 func TestClientCancelConnectionResultsHTTPCode499(t *testing.T) {
 	var timeoutStop sync.WaitGroup
 	timeoutStop.Add(1)

From df10d1c37a87041fa4eae75f9e68f5e85c4c27f3 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 16:43:26 +0000
Subject: [PATCH 09/19] Clear proxy error from context after balancer call so
 providers can check for previous error

---
 middleware/proxy.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 85f2f41c4..6516c913c 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -326,7 +326,12 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 				} else {
 					tgt = config.Balancer.Next(c)
 				}
+
+				//Clear any previous errors from context here so
+				//that balancers have the option to check for errors
+				//using previous target
 				c.Set(config.ContextKey, tgt)
+				c.Set("_error", nil)
 
 				// Proxy
 				switch {
@@ -348,11 +353,6 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 				}
 
 				retries--
-
-				//Try request again. Clear the previous error and target
-				//server from context.
-				c.Set("_error", nil)
-				c.Set(config.ContextKey, nil)
 			}
 		}
 	}

From f04fc6d0430691bdbec0fad0385657a85fab754a Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 16:52:28 +0000
Subject: [PATCH 10/19] Update proxyRaw to store actual Errors in _error
 context key in all cases

---
 middleware/proxy.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 6516c913c..83f37e63e 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -133,14 +133,14 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		in, _, err := c.Response().Hijack()
 		if err != nil {
-			c.Set("_error", fmt.Sprintf("proxy raw, hijack error=%v, url=%s", t.URL, err))
+			c.Set("_error", fmt.Errorf("proxy raw, hijack error=%w, url=%s", err, t.URL))
 			return
 		}
 		defer in.Close()
 
 		out, err := net.Dial("tcp", t.URL.Host)
 		if err != nil {
-			c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("proxy raw, dial error=%v, url=%s", t.URL, err)))
+			c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("proxy raw, dial error=%v, url=%s", err, t.URL)))
 			return
 		}
 		defer out.Close()
@@ -148,7 +148,7 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 		// Write header
 		err = r.Write(out)
 		if err != nil {
-			c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("proxy raw, request header copy error=%v, url=%s", t.URL, err)))
+			c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("proxy raw, request header copy error=%v, url=%s", err, t.URL)))
 			return
 		}
 
@@ -162,7 +162,7 @@ func proxyRaw(t *ProxyTarget, c echo.Context) http.Handler {
 		go cp(in, out)
 		err = <-errCh
 		if err != nil && err != io.EOF {
-			c.Set("_error", fmt.Errorf("proxy raw, copy body error=%v, url=%s", t.URL, err))
+			c.Set("_error", fmt.Errorf("proxy raw, copy body error=%w, url=%s", err, t.URL))
 		}
 	})
 }

From c3ad65794f6f6c6677fec5690a4aaf6f121fb56f Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 17:07:31 +0000
Subject: [PATCH 11/19] Fix linting errors

---
 middleware/proxy_test.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 5b53b76b6..e5344b4fa 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -406,24 +406,24 @@ func TestProxyRetries(t *testing.T) {
 		return targetUrl, server
 	}
 
-	targetUrl, server := newServer(http.StatusOK)
+	targetURL, server := newServer(http.StatusOK)
 	defer server.Close()
 	goodTarget := &ProxyTarget{
 		Name: "Good",
-		URL:  targetUrl,
+		URL:  targetURL,
 	}
 
-	targetUrl, server = newServer(http.StatusBadRequest)
+	targetURL, server = newServer(http.StatusBadRequest)
 	defer server.Close()
 	goodTargetWith40X := &ProxyTarget{
 		Name: "Good with 40X",
-		URL:  targetUrl,
+		URL:  targetURL,
 	}
 
-	targetUrl, _ = url.Parse("http://127.0.0.1:27121")
+	targetURL, _ = url.Parse("http://127.0.0.1:27121")
 	badTarget := &ProxyTarget{
 		Name: "Bad",
-		URL:  targetUrl,
+		URL:  targetURL,
 	}
 
 	alwaysRetryFilter := func(c echo.Context, e error) bool { return true }

From c4232f79b21c50c7e25ceb7f37a258ea324e524c Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 17:31:46 +0000
Subject: [PATCH 12/19] Doc updates for Proxy retry features

---
 middleware/proxy.go | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 83f37e63e..875b886fc 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -29,27 +29,29 @@ type (
 		// Required.
 		Balancer ProxyBalancer
 
-		// RetryCount defines the number of times a proxied request to an unavailable
-		// ProxyTarget should be retried using the next available ProxyTarget. Defaults
-		// to 0, meaning requests are never retried.
+		// RetryCount defines the number of times a failed proxied request should be retried
+		// using the next available ProxyTarget. Defaults to 0, meaning requests are never retried.
 		RetryCount int
 
-		// RetryFilter defines a function used to determine if a failed request to an
-		// unavailable ProxyTarget should be retried. The RetryFilter will only be called
-		// when the number of previous retries is less than RetryCount. If the function
-		// returns true, the request will be retried. When not specified, all requests that
-		// fail with http.StatusBadGateway error will be retried. A custom RetryFilter can
-		// be provided to only retry specific requests. Note that RetryFilter is only
-		// called when the request to the target fails, or an internal error in the Proxy
-		// middleware has occurred. Successful requests that return a non-200 response code
-		// cannot be retried.
+		// RetryFilter defines a function used to determine if a failed request to a
+		// ProxyTarget should be retried. The RetryFilter will only be called when the number
+		// of previous retries is less than RetryCount. If the function returns true, the
+		// request will be retried. The provided error indicates the reason for the request
+		// failure. When the ProxyTarget is unavailable, the error will be an instance of
+		// echo.HTTPError with a Code of http.StatusBadGateway. In all other cases, the error
+		// will indicate an internal error in the Proxy middleware. When a RetryFilter is not
+		// specified, all requests that fail with http.StatusBadGateway will be retried. A custom
+		// RetryFilter can be provided to only retry specific requests. Note that RetryFilter is
+		// only called when the request to the target fails, or an internal error in the Proxy
+		// middleware has occurred. Successful requests that return a non-200 response code cannot
+		// be retried.
 		RetryFilter func(c echo.Context, e error) bool
 
 		// ErrorHandler defines a function which can be used to return custom errors from
 		// the Proxy middleware. ErrorHandler is only invoked when there has been
 		// either an internal error in the Proxy middleware or the ProxyTarget is
 		// unavailable. Due to the way requests are proxied, ErrorHandler is not invoked
-		// when a Proxy target returns a non-200 response. In these cases, the response
+		// when a ProxyTarget returns a non-200 response. In these cases, the response
 		// is already written so errors cannot be modified. ErrorHandler is only
 		// invoked after all retry attempts have been exhausted.
 		ErrorHandler func(c echo.Context, err error) error

From 1cc7aa1f3df666c8f9248e9479256728201a2656 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 17:34:25 +0000
Subject: [PATCH 13/19] Remove redundant test comments

---
 middleware/proxy_test.go | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index e5344b4fa..ebce1a4a9 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -593,9 +593,7 @@ func TestProxyErrorHandler(t *testing.T) {
 	}
 
 	transformedError := errors.New("a new error")
-	//Not called when no error
-	//Called when error and returned error passed to central
-	//
+
 	testCases := []struct {
 		name             string
 		target           *ProxyTarget

From c6e358cf6e596ed407adae0a62d7bfafebde10f4 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Fri, 17 Mar 2023 17:44:26 +0000
Subject: [PATCH 14/19] only clear proxy error key when we need to

---
 middleware/proxy.go | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 875b886fc..5d3477b54 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -329,11 +329,14 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 					tgt = config.Balancer.Next(c)
 				}
 
-				//Clear any previous errors from context here so
-				//that balancers have the option to check for errors
-				//using previous target
 				c.Set(config.ContextKey, tgt)
-				c.Set("_error", nil)
+
+				//If retrying a failed request, clear any previous errors from
+				//context here so that balancers have the option to check for
+				//errors that occurred using previous target
+				if retries < config.RetryCount {
+					c.Set("_error", nil)
+				}
 
 				// Proxy
 				switch {

From dbbaaddd3437dabe9bd193a49327979f8797ca1d Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Thu, 23 Mar 2023 13:35:18 +0000
Subject: [PATCH 15/19] Fix linting errors

---
 middleware/proxy_test.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index ebce1a4a9..ef0126d46 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -402,8 +402,8 @@ func TestProxyRetries(t *testing.T) {
 				w.WriteHeader(res)
 			}),
 		)
-		targetUrl, _ := url.Parse(server.URL)
-		return targetUrl, server
+		targetURL, _ := url.Parse(server.URL)
+		return targetURL, server
 	}
 
 	targetURL, server := newServer(http.StatusOK)
@@ -579,17 +579,17 @@ func TestProxyErrorHandler(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
-	goodUrl, _ := url.Parse(server.URL)
+	goodURL, _ := url.Parse(server.URL)
 	defer server.Close()
 	goodTarget := &ProxyTarget{
 		Name: "Good",
-		URL:  goodUrl,
+		URL:  goodURL,
 	}
 
-	badUrl, _ := url.Parse("http://127.0.0.1:27121")
+	badURL, _ := url.Parse("http://127.0.0.1:27121")
 	badTarget := &ProxyTarget{
 		Name: "Bad",
-		URL:  badUrl,
+		URL:  badURL,
 	}
 
 	transformedError := errors.New("a new error")

From 179e74f91bdd903ec676f7b018545934f68cedb2 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Tue, 28 Mar 2023 10:36:33 +0100
Subject: [PATCH 16/19] Add test covering proxy retries on timeout

---
 middleware/proxy.go      |  1 +
 middleware/proxy_test.go | 56 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 5d3477b54..2fb36cb74 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -239,6 +239,7 @@ func (b *roundRobinBalancer) Next(c echo.Context) *ProxyTarget {
 	} else if len(b.targets) == 1 {
 		return b.targets[0]
 	}
+
 	// reset the index if out of bounds
 	if b.i >= len(b.targets) {
 		b.i = 0
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index ef0126d46..a9f130cfa 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -573,6 +573,62 @@ func TestProxyRetries(t *testing.T) {
 	}
 }
 
+func TestProxyRetryWithBackendTimeout(t *testing.T) {
+
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.ResponseHeaderTimeout = time.Millisecond * 500
+
+	timeoutBackend := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			time.Sleep(1 * time.Second)
+			w.WriteHeader(404)
+		}),
+	)
+	defer timeoutBackend.Close()
+
+	timeoutTargetURL, _ := url.Parse(timeoutBackend.URL)
+	goodBackend := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(200)
+		}),
+	)
+	defer goodBackend.Close()
+
+	goodTargetURL, _ := url.Parse(goodBackend.URL)
+	e := echo.New()
+	e.Use(ProxyWithConfig(
+		ProxyConfig{
+			Transport: transport,
+			Balancer: NewRoundRobinBalancer([]*ProxyTarget{
+				{
+					Name: "Timeout",
+					URL:  timeoutTargetURL,
+				},
+				{
+					Name: "Good",
+					URL:  goodTargetURL,
+				},
+			}),
+			RetryCount: 1,
+		},
+	))
+
+	var wg sync.WaitGroup
+	for i := 0; i < 20; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			rec := httptest.NewRecorder()
+			e.ServeHTTP(rec, req)
+			assert.Contains(t, []int{200, 502}, rec.Code)
+		}()
+	}
+
+	wg.Wait()
+
+}
+
 func TestProxyErrorHandler(t *testing.T) {
 
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

From f3472cd2a5365f9e70ad9cebfeec812baa176232 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Tue, 28 Mar 2023 12:15:19 +0100
Subject: [PATCH 17/19] Proxy round robin balancer uses next target for retried
 requests

---
 middleware/proxy.go      | 27 +++++++++++++++++++++------
 middleware/proxy_test.go |  2 +-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 2fb36cb74..619f7d63e 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -240,13 +240,28 @@ func (b *roundRobinBalancer) Next(c echo.Context) *ProxyTarget {
 		return b.targets[0]
 	}
 
-	// reset the index if out of bounds
-	if b.i >= len(b.targets) {
-		b.i = 0
+	var i int
+	const lastIdxKey = "_round_robin_last_index"
+	// This request is a retry, start from the index of the previous
+	// target to ensure we don't attempt to retry the request with
+	// the same failed target
+	if c.Get(lastIdxKey) != nil {
+		i = c.Get(lastIdxKey).(int)
+		i++
+		if i >= len(b.targets) {
+			i = 0
+		}
+		// This is a first time request, use the global index
+	} else {
+		i = b.i
+		b.i++
+		if b.i >= len(b.targets) {
+			b.i = 0
+		}
 	}
-	t := b.targets[b.i]
-	b.i++
-	return t
+
+	c.Set(lastIdxKey, i)
+	return b.targets[i]
 }
 
 // Proxy returns a Proxy middleware.
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index a9f130cfa..1b5ba6cbe 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -621,7 +621,7 @@ func TestProxyRetryWithBackendTimeout(t *testing.T) {
 			req := httptest.NewRequest(http.MethodGet, "/", nil)
 			rec := httptest.NewRecorder()
 			e.ServeHTTP(rec, req)
-			assert.Contains(t, []int{200, 502}, rec.Code)
+			assert.Equal(t, 200, rec.Code)
 		}()
 	}
 

From 7c2cd96f3fc302023e9c9f8113617f49c8a83870 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Tue, 4 Apr 2023 11:34:37 +0100
Subject: [PATCH 18/19] Fix potential range error when round robin balancer
 targets changed.

---
 middleware/proxy.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 619f7d63e..1fcc4ad00 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -251,13 +251,13 @@ func (b *roundRobinBalancer) Next(c echo.Context) *ProxyTarget {
 		if i >= len(b.targets) {
 			i = 0
 		}
-		// This is a first time request, use the global index
 	} else {
-		i = b.i
-		b.i++
+		// This is a first time request, use the global index
 		if b.i >= len(b.targets) {
 			b.i = 0
 		}
+		i = b.i
+		b.i++
 	}
 
 	c.Set(lastIdxKey, i)

From 55602543efe21838ba67910cae53f2a294578b93 Mon Sep 17 00:00:00 2001
From: mikemherron <15673068+mikemherron@users.noreply.github.com>
Date: Tue, 4 Apr 2023 11:36:14 +0100
Subject: [PATCH 19/19] Documented expected retry behaviour when RR balancer
 proxy targets change

---
 middleware/proxy.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/middleware/proxy.go b/middleware/proxy.go
index 1fcc4ad00..e4f98d9ed 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -228,7 +228,12 @@ func (b *randomBalancer) Next(c echo.Context) *ProxyTarget {
 	return b.targets[b.random.Intn(len(b.targets))]
 }
 
-// Next returns an upstream target using round-robin technique.
+// Next returns an upstream target using round-robin technique. In the case
+// where a previously failed request is being retried, the round-robin
+// balancer will attempt to use the next target relative to the original
+// request. If the list of targets held by the balancer is modified while a
+// failed request is being retried, it is possible that the balancer will
+// return the original failed target.
 //
 // Note: `nil` is returned in case upstream target list is empty.
 func (b *roundRobinBalancer) Next(c echo.Context) *ProxyTarget {