Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support retries of failed proxy requests #2414

Merged
merged 19 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
adb1901
Draft proposal for supporting retries of failed proxy requests when b…
mikemherron Mar 7, 2023
46e5bd5
Add retry count as first class feature, simplify callback, add tests
mikemherron Mar 10, 2023
7c35cda
Minor comment tidy up
mikemherron Mar 10, 2023
678da72
Rename retry handler to filter
mikemherron Mar 10, 2023
675c19e
use named fields for structs
mikemherron Mar 11, 2023
5989375
Remove function type for filter, don't use Default var for default
mikemherron Mar 11, 2023
c12daa7
Add error to RetryFilter, move BadGateway check to default implementa…
mikemherron Mar 17, 2023
7299721
Add ErrorHandler to Proxy middleware
mikemherron Mar 17, 2023
df10d1c
Clear proxy error from context after balancer call so providers can c…
mikemherron Mar 17, 2023
f04fc6d
Update proxyRaw to store actual Errors in _error context key in all c…
mikemherron Mar 17, 2023
c3ad657
Fix linting errors
mikemherron Mar 17, 2023
c4232f7
Doc updates for Proxy retry features
mikemherron Mar 17, 2023
1cc7aa1
Remove redundant test comments
mikemherron Mar 17, 2023
c6e358c
only clear proxy error key when we need to
mikemherron Mar 17, 2023
dbbaadd
Fix linting errors
mikemherron Mar 23, 2023
179e74f
Add test covering proxy retries on timeout
mikemherron Mar 28, 2023
f3472cd
Proxy round robin balancer uses next target for retried requests
mikemherron Mar 28, 2023
7c2cd96
Fix potential range error when round robin balancer targets changed.
mikemherron Apr 4, 2023
5560254
Documented expected retry behaviour when RR balancer proxy targets ch…
mikemherron Apr 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
155 changes: 115 additions & 40 deletions middleware/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,33 @@ type (
// Required.
Balancer ProxyBalancer

// 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 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 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

// Rewrite defines URL path rewrite rules. The values captured in asterisk can be
// retrieved by index e.g. $1, $2 and so on.
// Examples:
Expand Down Expand Up @@ -71,7 +98,8 @@ 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)
}
Expand Down Expand Up @@ -107,22 +135,22 @@ 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()

// 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
}

Expand All @@ -136,7 +164,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))
}
})
}
Expand Down Expand Up @@ -211,13 +239,29 @@ 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

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 {
aldas marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -232,14 +276,26 @@ 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.Balancer == nil {
panic("echo: proxy middleware requires balancer")
if config.RetryFilter == nil {
aldas marked this conversation as resolved.
Show resolved Hide resolved
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.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)
Expand All @@ -250,28 +306,17 @@ 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
return config.ErrorHandler(c, err)
}

// Fix header
Expand All @@ -287,19 +332,49 @@ 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
}
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)
}

return
c.Set(config.ContextKey, tgt)
mikemherron marked this conversation as resolved.
Show resolved Hide resolved

//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 {
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)
}

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

retry := retries > 0 && config.RetryFilter(c, err)
if !retry {
return config.ErrorHandler(c, err)
}

retries--
}
}
}
}
Expand Down