From 73b67b707fbfa682fcf83fc675ed8190797b7441 Mon Sep 17 00:00:00 2001 From: "d.anchikov" Date: Fri, 17 Jun 2022 16:54:25 +0400 Subject: [PATCH] feat: Added retry logic to remote api request --- api/handler/triggers.go | 2 +- clock/clock.go | 5 + cmd/config.go | 40 ++++- interfaces.go | 1 + local/checker.yml | 5 +- metric_source/remote/config.go | 17 +- metric_source/remote/remote.go | 37 +++-- metric_source/remote/remote_test.go | 185 +++++++++++++++++++--- metric_source/remote/request.go | 66 ++++++-- metric_source/remote/request_test.go | 222 ++++++++++++++++++++++++++- mock/clock/clock.go | 12 ++ 11 files changed, 532 insertions(+), 60 deletions(-) diff --git a/api/handler/triggers.go b/api/handler/triggers.go index f1a5987b6..017c62155 100644 --- a/api/handler/triggers.go +++ b/api/handler/triggers.go @@ -91,7 +91,7 @@ func getTriggerFromRequest(request *http.Request) (*dto.Trigger, *api.ErrorRespo return nil, api.ErrorInvalidRequest(err) case remote.ErrRemoteTriggerResponse: response := api.ErrorRemoteServerUnavailable(err) - middleware.GetLoggerEntry(request).Error("%s : %s : %s", response.StatusText, response.ErrorText, err) + middleware.GetLoggerEntry(request).Errorf("%s : %s : %s", response.StatusText, response.ErrorText, err) return nil, response default: return nil, api.ErrorInternalServer(err) diff --git a/clock/clock.go b/clock/clock.go index b5e253805..a2f80d37b 100644 --- a/clock/clock.go +++ b/clock/clock.go @@ -14,3 +14,8 @@ func NewSystemClock() *SystemClock { func (t *SystemClock) Now() time.Time { return time.Now().UTC() } + +// Sleep pauses the current goroutine for at least the passed duration +func (t *SystemClock) Sleep(duration time.Duration) { + time.Sleep(duration) +} diff --git a/cmd/config.go b/cmd/config.go index 02f7df4e2..84a303ecc 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -3,7 +3,9 @@ package cmd import ( "fmt" "io/ioutil" + "strconv" "strings" + "time" "github.com/moira-alert/moira/metrics" @@ -113,6 +115,12 @@ type RemoteConfig struct { MetricsTTL string `yaml:"metrics_ttl"` // Timeout for remote requests Timeout string `yaml:"timeout"` + // Retry seconds for remote requests divided by spaces + RetrySeconds string `yaml:"retry_seconds"` + // HealthCheckTimeout is timeout for remote api health check requests + HealthCheckTimeout string `yaml:"health_check_timeout"` + // Retry seconds for remote api health check requests divided by spaces + HealthCheckRetrySeconds string `yaml:"health_check_retry_seconds"` // Username for basic auth User string `yaml:"user"` // Password for basic auth @@ -129,16 +137,34 @@ type ImageStoreConfig struct { // GetRemoteSourceSettings returns remote config parsed from moira config files func (config *RemoteConfig) GetRemoteSourceSettings() *remoteSource.Config { return &remoteSource.Config{ - URL: config.URL, - CheckInterval: to.Duration(config.CheckInterval), - MetricsTTL: to.Duration(config.MetricsTTL), - Timeout: to.Duration(config.Timeout), - User: config.User, - Password: config.Password, - Enabled: config.Enabled, + URL: config.URL, + CheckInterval: to.Duration(config.CheckInterval), + MetricsTTL: to.Duration(config.MetricsTTL), + Timeout: to.Duration(config.Timeout), + RetrySeconds: ParseRetrySeconds(config.RetrySeconds), + HealthCheckTimeout: to.Duration(config.HealthCheckTimeout), + HealthCheckRetrySeconds: ParseRetrySeconds(config.HealthCheckRetrySeconds), + User: config.User, + Password: config.Password, + Enabled: config.Enabled, } } +// ParseRetrySeconds parses config value string into array of integers +func ParseRetrySeconds(retrySecondsString string) []time.Duration { + secondsStringList := strings.Fields(retrySecondsString) + retrySecondsIntList := make([]time.Duration, len(secondsStringList)) + + for index, secondsString := range secondsStringList { + secondsInt, err := strconv.Atoi(secondsString) + if err != nil { + panic(err) + } + retrySecondsIntList[index] = time.Second * time.Duration(secondsInt) + } + return retrySecondsIntList +} + // ReadConfig parses config file by the given path into Moira-used type func ReadConfig(configFileName string, config interface{}) error { configYaml, err := ioutil.ReadFile(configFileName) diff --git a/interfaces.go b/interfaces.go index 080986b2c..88cbf5dae 100644 --- a/interfaces.go +++ b/interfaces.go @@ -224,4 +224,5 @@ type PlotTheme interface { // Clock is an interface to work with Time. type Clock interface { Now() time.Time + Sleep(duration time.Duration) } diff --git a/local/checker.yml b/local/checker.yml index c704183ad..f92b9b55b 100644 --- a/local/checker.yml +++ b/local/checker.yml @@ -17,7 +17,10 @@ remote: url: "http://graphite:80/render" check_interval: 60s timeout: 60s - metrics_ttl: 7d + metrics_ttl: 168h + retry_seconds: 1 1 1 + health_check_timeout: 6s + health_check_retry_seconds: 1 1 1 checker: nodata_check_interval: 60s check_interval: 10s diff --git a/metric_source/remote/config.go b/metric_source/remote/config.go index 63278e13d..115ef2787 100644 --- a/metric_source/remote/config.go +++ b/metric_source/remote/config.go @@ -4,13 +4,16 @@ import "time" // Config represents config from remote storage type Config struct { - URL string - CheckInterval time.Duration - MetricsTTL time.Duration - Timeout time.Duration - User string - Password string - Enabled bool + URL string + CheckInterval time.Duration + MetricsTTL time.Duration + Timeout time.Duration + RetrySeconds []time.Duration + HealthCheckTimeout time.Duration + HealthCheckRetrySeconds []time.Duration + User string + Password string + Enabled bool } // isEnabled checks that remote config is enabled (url is defined and enabled flag is set) diff --git a/metric_source/remote/remote.go b/metric_source/remote/remote.go index 6be143ecf..ea93385a5 100644 --- a/metric_source/remote/remote.go +++ b/metric_source/remote/remote.go @@ -5,6 +5,8 @@ import ( "net/http" "time" + "github.com/moira-alert/moira/clock" + "github.com/moira-alert/moira" metricSource "github.com/moira-alert/moira/metric_source" ) @@ -23,10 +25,22 @@ func (err ErrRemoteTriggerResponse) Error() string { return err.InternalError.Error() } +// ErrRemoteUnavailable is a custom error when remote trigger check fails +type ErrRemoteUnavailable struct { + InternalError error + Target string +} + +// Error is a representation of Error interface method +func (err ErrRemoteUnavailable) Error() string { + return err.InternalError.Error() +} + // Remote is implementation of MetricSource interface, which implements fetch metrics method from remote graphite installation type Remote struct { config *Config client *http.Client + clock moira.Clock } // Create configures remote metric source @@ -34,6 +48,7 @@ func Create(config *Config) metricSource.MetricSource { return &Remote{ config: config, client: &http.Client{Timeout: config.Timeout}, + clock: clock.NewSystemClock(), } } @@ -50,9 +65,15 @@ func (remote *Remote) Fetch(target string, from, until int64, allowRealTimeAlert Target: target, } } - body, err := remote.makeRequest(req) + body, isRemoteAvailable, err := remote.makeRequestWithRetries(req, remote.config.Timeout, remote.config.RetrySeconds) if err != nil { - return nil, ErrRemoteTriggerResponse{ + if isRemoteAvailable { + return nil, ErrRemoteTriggerResponse{ + InternalError: err, + Target: target, + } + } + return nil, ErrRemoteUnavailable{ InternalError: err, Target: target, } @@ -83,18 +104,14 @@ func (remote *Remote) IsConfigured() (bool, error) { // IsRemoteAvailable checks if graphite API is available and returns 200 response func (remote *Remote) IsRemoteAvailable() (bool, error) { - maxRetries := 3 until := time.Now().Unix() from := until - 600 //nolint req, err := remote.prepareRequest(from, until, "NonExistingTarget") if err != nil { return false, err } - for attempt := 0; attempt < maxRetries; attempt++ { - _, err = remote.makeRequest(req) - if err == nil { - return true, nil - } - } - return false, err + _, isRemoteAvailable, err := remote.makeRequestWithRetries( + req, remote.config.HealthCheckTimeout, remote.config.HealthCheckRetrySeconds, + ) + return isRemoteAvailable, err } diff --git a/metric_source/remote/remote_test.go b/metric_source/remote/remote_test.go index 4828ef4ac..8a540bc6c 100644 --- a/metric_source/remote/remote_test.go +++ b/metric_source/remote/remote_test.go @@ -4,6 +4,11 @@ import ( "fmt" "net/http" "testing" + "time" + + "github.com/golang/mock/gomock" + + mock_clock "github.com/moira-alert/moira/mock/clock" metricSource "github.com/moira-alert/moira/metric_source" . "github.com/smartystreets/goconvey/convey" @@ -26,20 +31,79 @@ func TestIsConfigured(t *testing.T) { } func TestIsRemoteAvailable(t *testing.T) { - Convey("Is available", t, func() { - server := createServer([]byte("Some string"), http.StatusOK) - remote := Remote{client: server.Client(), config: &Config{URL: server.URL}} - isAvailable, err := remote.IsRemoteAvailable() - So(isAvailable, ShouldBeTrue) - So(err, ShouldBeEmpty) + mockCtrl := gomock.NewController(t) + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(0) + testConfigs := []Config{ + {}, + {HealthCheckRetrySeconds: []time.Duration{time.Second}}, + {HealthCheckRetrySeconds: []time.Duration{time.Second}}, + {HealthCheckRetrySeconds: []time.Duration{time.Second, time.Second}}, + {HealthCheckRetrySeconds: []time.Duration{time.Second, time.Second}}, + {HealthCheckRetrySeconds: []time.Duration{time.Second, time.Second, time.Second}}, + } + body := []byte("Some string") + + Convey("Given server returns OK response the remote is available", t, func() { + server := createServer(body, http.StatusOK) + for _, config := range testConfigs { + config.URL = server.URL + remote := Remote{client: server.Client(), config: &config} + isAvailable, err := remote.IsRemoteAvailable() + So(isAvailable, ShouldBeTrue) + So(err, ShouldBeEmpty) + } }) - Convey("Not available", t, func() { - server := createServer([]byte("Some string"), http.StatusInternalServerError) - remote := Remote{client: server.Client(), config: &Config{URL: server.URL}} - isAvailable, err := remote.IsRemoteAvailable() - So(isAvailable, ShouldBeFalse) - So(err, ShouldResemble, fmt.Errorf("bad response status %d: %s", http.StatusInternalServerError, "Some string")) + Convey("Given server returns Remote Unavailable responses permanently", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + server := createTestServer(TestResponse{body, statusCode}) + + Convey(fmt.Sprintf( + "request failed with %d response status code and remote is unavailable", statusCode, + ), func() { + remote := Remote{client: server.Client()} + for _, config := range testConfigs { + config.URL = server.URL + remote.config = &config + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(len(config.HealthCheckRetrySeconds)) + remote.clock = systemClock + + isAvailable, err := remote.IsRemoteAvailable() + So(err, ShouldResemble, fmt.Errorf( + "the remote server is not available. Response status %d: %s", statusCode, string(body), + )) + So(isAvailable, ShouldBeFalse) + } + }) + } + }) + + Convey("Given server returns Remote Unavailable response temporary", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + Convey(fmt.Sprintf( + "the remote is available with retry after %d response", statusCode, + ), func() { + for _, config := range testConfigs { + if len(config.HealthCheckRetrySeconds) == 0 { + continue + } + server := createTestServer( + TestResponse{body, statusCode}, + TestResponse{body, http.StatusOK}, + ) + config.URL = server.URL + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(1) + remote := Remote{client: server.Client(), config: &config, clock: systemClock} + + isAvailable, err := remote.IsRemoteAvailable() + So(err, ShouldBeNil) + So(isAvailable, ShouldBeTrue) + } + }) + } }) } @@ -47,6 +111,16 @@ func TestFetch(t *testing.T) { var from int64 = 300 var until int64 = 500 target := "foo.bar" //nolint + testConfigs := []Config{ + {}, + {RetrySeconds: []time.Duration{time.Second}}, + {RetrySeconds: []time.Duration{time.Second}}, + {RetrySeconds: []time.Duration{time.Second, time.Second}}, + {RetrySeconds: []time.Duration{time.Second, time.Second}}, + {RetrySeconds: []time.Duration{time.Second, time.Second, time.Second}}, + } + mockCtrl := gomock.NewController(t) + validBody := []byte("[{\"Target\": \"t1\",\"DataPoints\":[[1,2],[3,4]]}]") Convey("Request success but body is invalid", t, func() { server := createServer([]byte("[]"), http.StatusOK) @@ -62,21 +136,90 @@ func TestFetch(t *testing.T) { result, err := remote.Fetch(target, from, until, false) So(result, ShouldBeEmpty) So(err.Error(), ShouldResemble, "invalid character 'S' looking for beginning of value") + _, ok := err.(ErrRemoteTriggerResponse) + So(ok, ShouldBeTrue) }) Convey("Fail request with InternalServerError", t, func() { server := createServer([]byte("Some string"), http.StatusInternalServerError) - remote := Remote{client: server.Client(), config: &Config{URL: server.URL}} - result, err := remote.Fetch(target, from, until, false) - So(result, ShouldBeEmpty) - So(err.Error(), ShouldResemble, fmt.Sprintf("bad response status %d: %s", http.StatusInternalServerError, "Some string")) + remote := Remote{client: server.Client()} + for _, config := range testConfigs { + config.URL = server.URL + remote.config = &config + result, err := remote.Fetch(target, from, until, false) + So(result, ShouldBeEmpty) + So(err.Error(), ShouldResemble, fmt.Sprintf("bad response status %d: %s", http.StatusInternalServerError, "Some string")) + _, ok := err.(ErrRemoteTriggerResponse) + So(ok, ShouldBeTrue) + } }) - Convey("Fail make request", t, func() { + Convey("Client calls bad url", t, func() { url := "💩%$&TR" - remote := Remote{config: &Config{URL: url}} - result, err := remote.Fetch(target, from, until, false) - So(result, ShouldBeEmpty) - So(err.Error(), ShouldResemble, "parse \"💩%$&TR\": invalid URL escape \"%$&\"") + for _, config := range testConfigs { + config.URL = url + remote := Remote{config: &config} + result, err := remote.Fetch(target, from, until, false) + So(result, ShouldBeEmpty) + So(err.Error(), ShouldResemble, "parse \"💩%$&TR\": invalid URL escape \"%$&\"") + _, ok := err.(ErrRemoteTriggerResponse) + So(ok, ShouldBeTrue) + } + }) + + Convey("Given server returns Remote Unavailable responses permanently", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + server := createTestServer(TestResponse{validBody, statusCode}) + + Convey(fmt.Sprintf( + "request failed with %d response status code and remote is unavailable", statusCode, + ), func() { + remote := Remote{client: server.Client()} + for _, config := range testConfigs { + config.URL = server.URL + remote.config = &config + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(len(config.RetrySeconds)) + remote.clock = systemClock + + result, err := remote.Fetch(target, from, until, false) + So(err, ShouldResemble, ErrRemoteUnavailable{ + InternalError: fmt.Errorf( + "the remote server is not available. Response status %d: %s", statusCode, string(validBody), + ), Target: target, + }) + So(result, ShouldBeNil) + } + }) + } + }) + + Convey("Given server returns Remote Unavailable response temporary", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + Convey(fmt.Sprintf( + "the remote is available with retry after %d response", statusCode, + ), func() { + for _, config := range testConfigs { + if len(config.RetrySeconds) == 0 { + continue + } + server := createTestServer( + TestResponse{validBody, statusCode}, + TestResponse{validBody, http.StatusOK}, + ) + config.URL = server.URL + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(1) + remote := Remote{client: server.Client(), config: &config, clock: systemClock} + + result, err := remote.Fetch(target, from, until, false) + So(err, ShouldBeNil) + So(result, ShouldNotBeNil) + metricsData := result.GetMetricsData() + So(len(metricsData), ShouldEqual, 1) + So(metricsData[0].Name, ShouldEqual, "t1") + } + }) + } }) } diff --git a/metric_source/remote/request.go b/metric_source/remote/request.go index c6eb72e56..c877575e5 100644 --- a/metric_source/remote/request.go +++ b/metric_source/remote/request.go @@ -1,10 +1,12 @@ package remote import ( + "context" "fmt" "io/ioutil" "net/http" "strconv" + "time" ) func (remote *Remote) prepareRequest(from, until int64, target string) (*http.Request, error) { @@ -24,27 +26,73 @@ func (remote *Remote) prepareRequest(from, until int64, target string) (*http.Re return req, nil } -func (remote *Remote) makeRequest(req *http.Request) ([]byte, error) { - var body []byte - +func (remote *Remote) makeRequest(req *http.Request) (body []byte, isRemoteAvailable bool, err error) { resp, err := remote.client.Do(req) if resp != nil { defer resp.Body.Close() } if err != nil { - return body, fmt.Errorf("The remote server is not available or the response was reset by timeout. " + //nolint - "TTL: %s, PATH: %s, ERROR: %v ", remote.client.Timeout.String(), req.URL.RawPath, err) + return body, false, fmt.Errorf( + "the remote server is not available or the response was reset by timeout. PATH: %s, ERROR: %v ", + req.URL.RawPath, + err, + ) } body, err = ioutil.ReadAll(resp.Body) if err != nil { - return body, err + return body, false, err + } + + if isRemoteUnavailableStatusCode(resp.StatusCode) { + return body, false, fmt.Errorf( + "the remote server is not available. Response status %d: %s", resp.StatusCode, string(body), + ) + } else if resp.StatusCode != http.StatusOK { + return body, true, fmt.Errorf("bad response status %d: %s", resp.StatusCode, string(body)) + } + + return body, true, nil +} + +func isRemoteUnavailableStatusCode(statusCode int) bool { + switch statusCode { + case http.StatusUnauthorized, + http.StatusBadGateway, + http.StatusServiceUnavailable, + http.StatusGatewayTimeout: + return true + default: + return false } +} - if resp.StatusCode != 200 { //nolint - return body, fmt.Errorf("bad response status %d: %s", resp.StatusCode, string(body)) +func (remote *Remote) makeRequestWithRetries( + req *http.Request, + requestTimeout time.Duration, + retrySeconds []time.Duration, +) (body []byte, isRemoteAvailable bool, err error) { + for attemptIndex := 0; attemptIndex < len(retrySeconds)+1; attemptIndex++ { + body, isRemoteAvailable, err = remote.makeRequestWithTimeout(req, requestTimeout) + if err == nil || isRemoteAvailable { + return body, true, err + } + if attemptIndex < len(retrySeconds) { + remote.clock.Sleep(retrySeconds[attemptIndex]) + } } + return nil, false, err +} - return body, nil +func (remote *Remote) makeRequestWithTimeout( + req *http.Request, + requestTimeout time.Duration, +) (body []byte, isRemoteAvailable bool, err error) { + if requestTimeout > 0 { + ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) + defer cancel() + req = req.WithContext(ctx) + } + return remote.makeRequest(req) } diff --git a/metric_source/remote/request_test.go b/metric_source/remote/request_test.go index 340a8cef9..ecd9e057f 100644 --- a/metric_source/remote/request_test.go +++ b/metric_source/remote/request_test.go @@ -5,6 +5,10 @@ import ( "net/http" "net/http/httptest" "testing" + "time" + + "github.com/golang/mock/gomock" + mock_clock "github.com/moira-alert/moira/mock/clock" . "github.com/smartystreets/goconvey/convey" ) @@ -54,8 +58,9 @@ func TestMakeRequest(t *testing.T) { server := createServer(body, http.StatusOK) remote := Remote{client: server.Client(), config: &Config{URL: server.URL}} request, _ := remote.prepareRequest(from, until, target) - actual, err := remote.makeRequest(request) + actual, isRemoteAvailable, err := remote.makeRequest(request) So(err, ShouldBeNil) + So(isRemoteAvailable, ShouldBeTrue) So(actual, ShouldResemble, body) }) @@ -63,19 +68,189 @@ func TestMakeRequest(t *testing.T) { server := createServer(body, http.StatusInternalServerError) remote := Remote{client: server.Client(), config: &Config{URL: server.URL}} request, _ := remote.prepareRequest(from, until, target) - actual, err := remote.makeRequest(request) + actual, isRemoteAvailable, err := remote.makeRequest(request) So(err, ShouldResemble, fmt.Errorf("bad response status %d: %s", http.StatusInternalServerError, string(body))) + So(isRemoteAvailable, ShouldBeTrue) So(actual, ShouldResemble, body) }) Convey("Client calls bad url", t, func() { server := createServer(body, http.StatusOK) - remote := Remote{client: server.Client(), config: &Config{URL: "http://bad/"}} + client := server.Client() + client.Timeout = time.Millisecond + remote := Remote{client: client, config: &Config{URL: "http://bad/"}} request, _ := remote.prepareRequest(from, until, target) - actual, err := remote.makeRequest(request) + actual, isRemoteAvailable, err := remote.makeRequest(request) So(err, ShouldNotBeEmpty) + So(isRemoteAvailable, ShouldBeFalse) So(actual, ShouldBeEmpty) }) + + Convey("Client returns status Remote Unavailable status codes", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + server := createServer(body, statusCode) + remote := Remote{client: server.Client(), config: &Config{URL: server.URL}} + request, _ := remote.prepareRequest(from, until, target) + actual, isRemoteAvailable, err := remote.makeRequest(request) + So(err, ShouldResemble, fmt.Errorf( + "the remote server is not available. Response status %d: %s", statusCode, string(body), + )) + So(isRemoteAvailable, ShouldBeFalse) + So(actual, ShouldResemble, body) + } + }) +} + +func TestMakeRequestWithRetries(t *testing.T) { + var from int64 = 300 + var until int64 = 500 + target := "foo.bar" + body := []byte("Some string") + testConfigs := []Config{ + {}, + {RetrySeconds: []time.Duration{time.Second}}, + {RetrySeconds: []time.Duration{time.Second}}, + {RetrySeconds: []time.Duration{time.Second, time.Second}}, + {RetrySeconds: []time.Duration{time.Second, time.Second}}, + {RetrySeconds: []time.Duration{time.Second, time.Second, time.Second}}, + } + mockCtrl := gomock.NewController(t) + + Convey("Given server returns OK response", t, func() { + server := createTestServer(TestResponse{body, http.StatusOK}) + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(0) + + Convey("request is successful", func() { + remote := Remote{client: server.Client(), clock: systemClock} + + for _, config := range testConfigs { + config.URL = server.URL + remote.config = &config + request, _ := remote.prepareRequest(from, until, target) + actual, isRemoteAvailable, err := remote.makeRequestWithRetries( + request, + remote.config.Timeout, + remote.config.RetrySeconds, + ) + So(err, ShouldBeNil) + So(isRemoteAvailable, ShouldBeTrue) + So(actual, ShouldResemble, body) + } + }) + }) + + Convey("Given server returns 500 response", t, func() { + server := createTestServer(TestResponse{body, http.StatusInternalServerError}) + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(0) + + Convey("request failed with 500 response and remote is available", func() { + remote := Remote{client: server.Client(), clock: systemClock} + + for _, config := range testConfigs { + config.URL = server.URL + remote.config = &config + request, _ := remote.prepareRequest(from, until, target) + actual, isRemoteAvailable, err := remote.makeRequestWithRetries( + request, + remote.config.Timeout, + remote.config.RetrySeconds, + ) + So(err, ShouldResemble, fmt.Errorf("bad response status %d: %s", http.StatusInternalServerError, string(body))) + So(isRemoteAvailable, ShouldBeTrue) + So(actual, ShouldResemble, body) + } + }) + }) + + Convey("Given client calls bad url", t, func() { + server := createTestServer(TestResponse{body, http.StatusOK}) + + Convey("request failed and remote is unavailable", func() { + remote := Remote{client: server.Client()} + for _, config := range testConfigs { + config.URL = "http://bad/" + remote.config = &config + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(len(config.RetrySeconds)) + remote.clock = systemClock + + request, _ := remote.prepareRequest(from, until, target) + actual, isRemoteAvailable, err := remote.makeRequestWithRetries( + request, + time.Millisecond, + remote.config.RetrySeconds, + ) + So(err, ShouldNotBeEmpty) + So(isRemoteAvailable, ShouldBeFalse) + So(actual, ShouldBeEmpty) + } + }) + }) + + Convey("Given server returns Remote Unavailable responses permanently", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + server := createTestServer(TestResponse{body, statusCode}) + + Convey(fmt.Sprintf( + "request failed with %d response status code and remote is unavailable", statusCode, + ), func() { + remote := Remote{client: server.Client()} + for _, config := range testConfigs { + config.URL = server.URL + remote.config = &config + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(len(config.RetrySeconds)) + remote.clock = systemClock + + request, _ := remote.prepareRequest(from, until, target) + actual, isRemoteAvailable, err := remote.makeRequestWithRetries( + request, + remote.config.Timeout, + remote.config.RetrySeconds, + ) + So(err, ShouldResemble, fmt.Errorf( + "the remote server is not available. Response status %d: %s", statusCode, string(body), + )) + So(isRemoteAvailable, ShouldBeFalse) + So(actual, ShouldBeNil) + } + }) + } + }) + + Convey("Given server returns Remote Unavailable response temporary", t, func() { + for _, statusCode := range remoteUnavailableStatusCodes { + Convey(fmt.Sprintf( + "request is successful with retry after %d response and remote is available", statusCode, + ), func() { + for _, config := range testConfigs { + if len(config.RetrySeconds) == 0 { + continue + } + server := createTestServer( + TestResponse{body, statusCode}, + TestResponse{body, http.StatusOK}, + ) + config.URL = server.URL + systemClock := mock_clock.NewMockClock(mockCtrl) + systemClock.EXPECT().Sleep(time.Second).Times(1) + remote := Remote{client: server.Client(), config: &config, clock: systemClock} + + request, _ := remote.prepareRequest(from, until, target) + actual, isRemoteAvailable, err := remote.makeRequestWithRetries( + request, + remote.config.Timeout, + remote.config.RetrySeconds, + ) + So(err, ShouldBeNil) + So(isRemoteAvailable, ShouldBeTrue) + So(actual, ShouldResemble, body) + } + }) + } + }) } func createServer(body []byte, statusCode int) *httptest.Server { @@ -84,3 +259,42 @@ func createServer(body []byte, statusCode int) *httptest.Server { rw.Write(body) //nolint })) } + +func createTestServer(testResponses ...TestResponse) *httptest.Server { + responseWriter := NewTestResponseWriter(testResponses) + return httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + response := responseWriter.GetResponse() + rw.WriteHeader(response.statusCode) + rw.Write(response.body) //nolint + })) +} + +type TestResponse struct { + body []byte + statusCode int +} + +type TestResponseWriter struct { + responses []TestResponse + count int +} + +func NewTestResponseWriter(testResponses []TestResponse) *TestResponseWriter { + responseWriter := new(TestResponseWriter) + responseWriter.responses = testResponses + responseWriter.count = 0 + return responseWriter +} + +func (responseWriter *TestResponseWriter) GetResponse() TestResponse { + response := responseWriter.responses[responseWriter.count%len(responseWriter.responses)] + responseWriter.count++ + return response +} + +var remoteUnavailableStatusCodes = []int{ + http.StatusUnauthorized, + http.StatusBadGateway, + http.StatusServiceUnavailable, + http.StatusGatewayTimeout, +} diff --git a/mock/clock/clock.go b/mock/clock/clock.go index fbfb56afb..6b55a6f3a 100644 --- a/mock/clock/clock.go +++ b/mock/clock/clock.go @@ -47,3 +47,15 @@ func (mr *MockClockMockRecorder) Now() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Now", reflect.TypeOf((*MockClock)(nil).Now)) } + +// Sleep mocks base method. +func (m *MockClock) Sleep(arg0 time.Duration) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Sleep", arg0) +} + +// Sleep indicates an expected call of Sleep. +func (mr *MockClockMockRecorder) Sleep(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sleep", reflect.TypeOf((*MockClock)(nil).Sleep), arg0) +}