Skip to content

Commit

Permalink
Merge pull request #99 from kaleido-io/ffresty-require-code-configura…
Browse files Browse the repository at this point in the history
…tion

Add config option to provide a regex to decide status code that can be retried
  • Loading branch information
nguyer authored Sep 14, 2023
2 parents 8401dd6 + 3da3888 commit ba4df60
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/ffresty/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ const (
HTTPConfigRetryInitDelay = "retry.initWaitTime"
// HTTPConfigRetryMaxDelay the maximum retry delay
HTTPConfigRetryMaxDelay = "retry.maxWaitTime"
// HTTPConfigRetryErrorStatusCodeRegex the regex that the error response status code must match to trigger retry
HTTPConfigRetryErrorStatusCodeRegex = "retry.errorStatusCodeRegex"

// HTTPConfigRequestTimeout the request timeout
HTTPConfigRequestTimeout = "requestTimeout"
// HTTPIdleTimeout the max duration to hold a HTTP keepalive connection between calls
Expand Down Expand Up @@ -88,6 +91,7 @@ func InitConfig(conf config.Section) {
conf.AddKnownKey(HTTPConfigRetryCount, defaultRetryCount)
conf.AddKnownKey(HTTPConfigRetryInitDelay, defaultRetryWaitTime)
conf.AddKnownKey(HTTPConfigRetryMaxDelay, defaultRetryMaxWaitTime)
conf.AddKnownKey(HTTPConfigRetryErrorStatusCodeRegex)
conf.AddKnownKey(HTTPConfigRequestTimeout, defaultRequestTimeout)
conf.AddKnownKey(HTTPIdleTimeout, defaultHTTPIdleTimeout)
conf.AddKnownKey(HTTPMaxIdleConns, defaultHTTPMaxIdleConns)
Expand All @@ -113,6 +117,7 @@ func GenerateConfig(ctx context.Context, conf config.Section) (*Config, error) {
RetryCount: conf.GetInt(HTTPConfigRetryCount),
RetryInitialDelay: conf.GetDuration(HTTPConfigRetryInitDelay),
RetryMaximumDelay: conf.GetDuration(HTTPConfigRetryMaxDelay),
RetryErrorStatusCodeRegex: conf.GetString(HTTPConfigRetryErrorStatusCodeRegex),
HTTPRequestTimeout: conf.GetDuration(HTTPConfigRequestTimeout),
HTTPIdleConnTimeout: conf.GetDuration(HTTPIdleTimeout),
HTTPMaxIdleConns: conf.GetInt(HTTPMaxIdleConns),
Expand Down
2 changes: 2 additions & 0 deletions pkg/ffresty/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func TestWSConfigGeneration(t *testing.T) {
utConf.Set(HTTPConfigRetryInitDelay, 1)
utConf.Set(HTTPConfigRetryMaxDelay, 1)
utConf.Set(HTTPConfigRetryCount, 1)
utConf.Set(HTTPConfigRetryErrorStatusCodeRegex, "(?:429|503)")
utConf.Set(HTTPConfigRequestTimeout, 1)
utConf.Set(HTTPIdleTimeout, 1)
utConf.Set(HTTPMaxIdleConns, 1)
Expand All @@ -57,6 +58,7 @@ func TestWSConfigGeneration(t *testing.T) {
assert.Equal(t, "pass", config.AuthPassword)
assert.Equal(t, time.Duration(1000000), config.RetryInitialDelay)
assert.Equal(t, time.Duration(1000000), config.RetryMaximumDelay)
assert.Equal(t, "(?:429|503)", config.RetryErrorStatusCodeRegex)
assert.Equal(t, 1, config.RetryCount)
assert.Equal(t, true, config.Retry)
assert.Equal(t, true, config.HTTPPassthroughHeadersEnabled)
Expand Down
13 changes: 13 additions & 0 deletions pkg/ffresty/ffresty.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io"
"net"
"net/http"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -58,6 +59,7 @@ type Config struct {
RetryCount int `json:"retryCount,omitempty"`
RetryInitialDelay time.Duration `json:"retryInitialDelay,omitempty"`
RetryMaximumDelay time.Duration `json:"retryMaximumDelay,omitempty"`
RetryErrorStatusCodeRegex string `json:"retryErrorStatusCodeRegex,omitempty"`
HTTPMaxIdleConns int `json:"maxIdleConns,omitempty"`
HTTPMaxConnsPerHost int `json:"maxConnsPerHost,omitempty"`
HTTPPassthroughHeadersEnabled bool `json:"httpPassthroughHeadersEnabled,omitempty"`
Expand Down Expand Up @@ -208,6 +210,11 @@ func NewWithConfig(ctx context.Context, ffrestyConfig Config) (client *resty.Cli
client.SetHeader("Authorization", fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", ffrestyConfig.AuthUsername, ffrestyConfig.AuthPassword)))))
}

var retryStatusCodeRegex *regexp.Regexp
if ffrestyConfig.RetryErrorStatusCodeRegex != "" {
retryStatusCodeRegex = regexp.MustCompile(ffrestyConfig.RetryErrorStatusCodeRegex)
}

if ffrestyConfig.Retry {
retryCount := ffrestyConfig.RetryCount
minTimeout := ffrestyConfig.RetryInitialDelay
Expand All @@ -220,6 +227,12 @@ func NewWithConfig(ctx context.Context, ffrestyConfig Config) (client *resty.Cli
if r == nil || r.IsSuccess() {
return false
}

if r.StatusCode() > 0 && retryStatusCodeRegex != nil && !retryStatusCodeRegex.MatchString(r.Status()) {
// the error status code doesn't match the retry status code regex, stop retry
return false
}

rCtx := r.Request.Context()
rc := rCtx.Value(retryCtxKey{}).(*retryCtx)
if ffrestyConfig.OnCheckRetry != nil && !ffrestyConfig.OnCheckRetry(r, err) {
Expand Down
50 changes: 50 additions & 0 deletions pkg/ffresty/ffresty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"crypto/x509/pkix"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"log"
"math/big"
Expand Down Expand Up @@ -114,6 +115,55 @@ func TestRequestRetry(t *testing.T) {

}

func TestRequestRetryErrorStatusCodeRegex(t *testing.T) {

ctx := context.Background()

resetConf()
utConf.Set(HTTPConfigURL, "http://localhost:12345")
utConf.Set(HTTPConfigRetryEnabled, true)
utConf.Set(HTTPConfigRetryInitDelay, 1)
utConf.Set(HTTPConfigRetryErrorStatusCodeRegex, "(?:429|503)")

c, err := New(ctx, utConf)
assert.Nil(t, err)
httpmock.ActivateNonDefault(c.GetClient())
defer httpmock.DeactivateAndReset()

httpmock.RegisterResponder("GET", "http://localhost:12345/test",
httpmock.NewStringResponder(500, `{"message": "pop"}`))

httpmock.RegisterResponder("GET", "http://localhost:12345/test2",
httpmock.NewStringResponder(429, `{"message": "pop"}`))

httpmock.RegisterResponder("GET", "http://localhost:12345/test3",
httpmock.NewErrorResponder(errors.New("not http response")))

resp, err := c.R().Get("/test")
assert.NoError(t, err)
assert.Equal(t, 500, resp.StatusCode())
assert.Equal(t, 1, httpmock.GetTotalCallCount())

err = WrapRestErr(ctx, resp, err, i18n.MsgConfigFailed)
assert.Error(t, err)

resp, err = c.R().Get("/test2")
assert.NoError(t, err)
assert.Equal(t, 429, resp.StatusCode())
assert.Equal(t, 7, httpmock.GetTotalCallCount())

err = WrapRestErr(ctx, resp, err, i18n.MsgConfigFailed)
assert.Error(t, err)

resp, err = c.R().Get("/test3")
assert.Error(t, err)
assert.Equal(t, 0, resp.StatusCode())
assert.Equal(t, 13, httpmock.GetTotalCallCount())

err = WrapRestErr(ctx, resp, err, i18n.MsgConfigFailed)
assert.Error(t, err)

}
func TestConfWithProxy(t *testing.T) {

ctx := context.Background()
Expand Down

0 comments on commit ba4df60

Please sign in to comment.