Repository: labstack/echo
File: middleware/proxy.go (Go)

Code before:
			}

			retries := config.RetryCount
			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)
				}

				e, hasError := c.Get("_error").(error)
				if !hasError {
					return nil
				}

				retry := false
				if httpErr, ok := e.(*echo.HTTPError); ok {
					if httpErr.Code == http.StatusBadGateway {
						retry = retries > 0 && config.RetryFilter(c)
					}
				}

				if !retry {
					return e
				}

				retries--

				//Try request again. Clear the previous error and target
				//server from context.
				c.Set("_error", nil)
				c.Set(config.ContextKey, nil)
			}
		}
	}
}

Review comment (bug):
I would change this block to:

```go
				err, hasError := c.Get("_error").(error)
				if !hasError {
					return nil
				}

				if rErr := config.ErrorHandler(err, c); rErr != nil {
					return rErr
				}

				if retries <= 0 {
					return err
				}

				retries--
			}
```

~~I looked into `proxyHTTP` function and from that I think that checking `httpErr.Code == http.StatusBadGateway` is making too much assumptions in behalf of the developer.  Default behavior to  stop on every error is aligned how `proxyHTTP` works. It can return 2 variants `echo.NewHTTPError(StatusCodeContextCanceled` or `echo.NewHTTPError(http.StatusBadGateway` and all these cases are perfectly reasonable for not retrying anymore.~~

I removed `c.Set(config.ContextKey, nil)` because this will be always overridden anyway. Also not clearing target here allows developer to check which target fails when `tgt, err = provider.NextTarget(c)` is called. as at that time that target would be still in context.

and `c.Set("_error", nil)` was moved up so `provider.NextTarget(c)` could check it and after that we clear error from context.


Code after:
	}
	if config.Rewrite != nil {
		if config.RegexRewrite == nil {
			config.RegexRewrite = make(map[*regexp.Regexp]string)
		}
		for k, v := range rewriteRulesRegex(config.Rewrite) {
			config.RegexRewrite[k] = v
		}
	}

	provider, isTargetProvider := config.Balancer.(TargetProvider)

	return func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			if config.Skipper(c) {
				return next(c)
			}

			req := c.Request()
			res := c.Response()
			if err := rewriteURL(config.RegexRewrite, req); err != nil {
				return config.ErrorHandler(c, err)
			}

			// Fix header
			// Basically it's not good practice to unconditionally pass incoming x-real-ip header to upstream.
			// However, for backward compatibility, legacy behavior is preserved unless you configure Echo#IPExtractor.
			if req.Header.Get(echo.HeaderXRealIP) == "" || c.Echo().IPExtractor != nil {
				req.Header.Set(echo.HeaderXRealIP, c.RealIP())
			}
			if req.Header.Get(echo.HeaderXForwardedProto) == "" {
				req.Header.Set(echo.HeaderXForwardedProto, c.Scheme())
			}
			if c.IsWebSocket() && req.Header.Get(echo.HeaderXForwardedFor) == "" { // For HTTP, it is automatically set by Go HTTP reverse proxy.
				req.Header.Set(echo.HeaderXForwardedFor, c.RealIP())
			}

			retries := config.RetryCount
			for {
				var tgt *ProxyTarget
				var err error
				if isTargetProvider {
					tgt, err = provider.NextTarget(c)
					if err != nil {
						return config.ErrorHandler(c, err)
					}
				} else {
					tgt = config.Balancer.Next(c)
				}

				c.Set(config.ContextKey, tgt)