From 3e9c8a91f08eab9129b10a317cccf98ca904bdbe Mon Sep 17 00:00:00 2001 From: galactica Date: Thu, 30 Jan 2025 12:59:43 +0200 Subject: [PATCH 1/6] Add Retry --- internal/wrappers/client.go | 18 ++++++++++++ internal/wrappers/projects-http.go | 44 ++++++++++++++++++++++++------ internal/wrappers/scans-http.go | 42 +++++++++++++++++++++++----- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/internal/wrappers/client.go b/internal/wrappers/client.go index 65675211f..4ff6d5af5 100644 --- a/internal/wrappers/client.go +++ b/internal/wrappers/client.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "io/ioutil" + "math" "net" "net/http" "net/http/httptrace" @@ -70,6 +71,23 @@ var cachedAccessToken string var cachedAccessTime time.Time var Domains = make(map[string]struct{}) +func retryHTTPRequest(fn func() (*http.Response, error), retries int, delay time.Duration) (*http.Response, error) { + var lastErr error + for i := 0; i < retries; i++ { + resp, err := fn() + if err != nil { + lastErr = err + if resp != nil && resp.StatusCode == http.StatusBadGateway { + time.Sleep(time.Duration(math.Pow(2, float64(i))) * delay) + continue + } + return nil, err + } + return resp, nil + } + return nil, lastErr +} + func setAgentName(req *http.Request) { agentStr := viper.GetString(commonParams.AgentNameKey) + "/" + commonParams.Version req.Header.Set("User-Agent", agentStr) diff --git a/internal/wrappers/projects-http.go b/internal/wrappers/projects-http.go index 8ed7bbd60..ea311dfda 100644 --- a/internal/wrappers/projects-http.go +++ b/internal/wrappers/projects-http.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "time" "github.com/pkg/errors" "github.com/spf13/viper" @@ -30,7 +31,10 @@ func (p *ProjectsHTTPWrapper) Create(model *Project) (*ProjectResponseModel, *Er return nil, nil, err } - resp, err := SendHTTPRequest(http.MethodPost, p.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodPost, p.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -49,7 +53,10 @@ func (p *ProjectsHTTPWrapper) Update(projectID string, model *Project) error { return nil } - resp, err := SendHTTPRequest(http.MethodPut, fmt.Sprintf("%s/%s", p.path, projectID), bytes.NewBuffer(jsonBytes), true, clientTimeout) + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodPut, fmt.Sprintf("%s/%s", p.path, projectID), bytes.NewBuffer(jsonBytes), true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return err } @@ -79,7 +86,10 @@ func (p *ProjectsHTTPWrapper) UpdateConfiguration(projectID string, configuratio commonParams.ProjectIDFlag: projectID, } - resp, err := SendHTTPRequestWithQueryParams(http.MethodPatch, "api/configuration/project", params, bytes.NewBuffer(jsonBytes), clientTimeout) + fn := func() (*http.Response, error) { + return SendHTTPRequestWithQueryParams(http.MethodPatch, "api/configuration/project", params, bytes.NewBuffer(jsonBytes), clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, err } @@ -100,7 +110,10 @@ func (p *ProjectsHTTPWrapper) Get(params map[string]string) ( params[limit] = limitValue } - resp, err := SendHTTPRequestWithQueryParams(http.MethodGet, p.path, params, nil, clientTimeout) + fn := func() (*http.Response, error) { + return SendHTTPRequestWithQueryParams(http.MethodGet, p.path, params, nil, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -137,7 +150,11 @@ func (p *ProjectsHTTPWrapper) GetByID(projectID string) ( *ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequest(http.MethodGet, p.path+"/"+projectID, http.NoBody, true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodGet, p.path+"/"+projectID, http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -180,7 +197,11 @@ func (p *ProjectsHTTPWrapper) GetBranchesByID(projectID string, params map[strin var request = "/branches?project-id=" + projectID params["limit"] = limitValue - resp, err := SendHTTPRequestWithQueryParams(http.MethodGet, p.path+request, params, nil, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequestWithQueryParams(http.MethodGet, p.path+request, params, nil, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -215,7 +236,10 @@ func (p *ProjectsHTTPWrapper) GetBranchesByID(projectID string, params map[strin func (p *ProjectsHTTPWrapper) Delete(projectID string) (*ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequest(http.MethodDelete, p.path+"/"+projectID, http.NoBody, true, clientTimeout) + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodDelete, p.path+"/"+projectID, http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, err } @@ -232,7 +256,11 @@ func (p *ProjectsHTTPWrapper) Tags() ( *ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequest(http.MethodGet, p.path+"/tags", http.NoBody, true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodGet, p.path+"/tags", http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } diff --git a/internal/wrappers/scans-http.go b/internal/wrappers/scans-http.go index fc65bedb1..8c2067f66 100644 --- a/internal/wrappers/scans-http.go +++ b/internal/wrappers/scans-http.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "time" commonParams "github.com/checkmarx/ast-cli/internal/params" "github.com/pkg/errors" @@ -35,7 +36,11 @@ func (s *ScansHTTPWrapper) Create(model *Scan) (*ScanResponseModel, *ErrorModel, if err != nil { return nil, nil, err } - resp, err := SendHTTPRequest(http.MethodPost, s.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodPost, s.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -49,7 +54,11 @@ func (s *ScansHTTPWrapper) Create(model *Scan) (*ScanResponseModel, *ErrorModel, func (s *ScansHTTPWrapper) Get(params map[string]string) (*ScansCollectionResponseModel, *ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequestWithQueryParams(http.MethodGet, s.path, params, nil, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequestWithQueryParams(http.MethodGet, s.path, params, nil, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -85,7 +94,11 @@ func (s *ScansHTTPWrapper) Get(params map[string]string) (*ScansCollectionRespon func (s *ScansHTTPWrapper) GetByID(scanID string) (*ScanResponseModel, *ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequest(http.MethodGet, s.path+"/"+scanID, http.NoBody, true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodGet, s.path+"/"+scanID, http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -100,7 +113,11 @@ func (s *ScansHTTPWrapper) GetByID(scanID string) (*ScanResponseModel, *ErrorMod func (s *ScansHTTPWrapper) GetWorkflowByID(scanID string) ([]*ScanTaskResponseModel, *ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) path := fmt.Sprintf("%s/%s/workflow", s.path, scanID) - resp, err := SendHTTPRequest(http.MethodGet, path, http.NoBody, true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodGet, path, http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -141,7 +158,11 @@ func handleWorkflowResponseWithBody(resp *http.Response, err error) ([]*ScanTask func (s *ScansHTTPWrapper) Delete(scanID string) (*ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequest(http.MethodDelete, s.path+"/"+scanID, http.NoBody, true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodDelete, s.path+"/"+scanID, http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, err } @@ -162,7 +183,10 @@ func (s *ScansHTTPWrapper) Cancel(scanID string) (*ErrorModel, error) { return nil, err } - resp, err := SendHTTPRequest(http.MethodPatch, s.path+"/"+scanID, bytes.NewBuffer(b), true, clientTimeout) + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodPatch, s.path+"/"+scanID, bytes.NewBuffer(b), true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, err } @@ -176,7 +200,11 @@ func (s *ScansHTTPWrapper) Cancel(scanID string) (*ErrorModel, error) { func (s *ScansHTTPWrapper) Tags() (map[string][]string, *ErrorModel, error) { clientTimeout := viper.GetUint(commonParams.ClientTimeoutKey) - resp, err := SendHTTPRequest(http.MethodGet, s.path+"/tags", http.NoBody, true, clientTimeout) + + fn := func() (*http.Response, error) { + return SendHTTPRequest(http.MethodGet, s.path+"/tags", http.NoBody, true, clientTimeout) + } + resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) if err != nil { return nil, nil, err } From f8bb246a19fa36078fc3e02b6fb5aadcd1da6012 Mon Sep 17 00:00:00 2001 From: galactica Date: Thu, 30 Jan 2025 14:15:55 +0200 Subject: [PATCH 2/6] Retry for 4 times. and add UT --- internal/wrappers/client.go | 4 +- internal/wrappers/projects-http.go | 16 ++++---- internal/wrappers/scans-http.go | 14 +++---- internal/wrappers/scans-http_test.go | 61 ++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 17 deletions(-) create mode 100644 internal/wrappers/scans-http_test.go diff --git a/internal/wrappers/client.go b/internal/wrappers/client.go index 4ff6d5af5..021afb247 100644 --- a/internal/wrappers/client.go +++ b/internal/wrappers/client.go @@ -71,14 +71,14 @@ var cachedAccessToken string var cachedAccessTime time.Time var Domains = make(map[string]struct{}) -func retryHTTPRequest(fn func() (*http.Response, error), retries int, delay time.Duration) (*http.Response, error) { +func retryHTTPRequest(fn func() (*http.Response, error), retries int, delayInMillis time.Duration) (*http.Response, error) { var lastErr error for i := 0; i < retries; i++ { resp, err := fn() if err != nil { lastErr = err if resp != nil && resp.StatusCode == http.StatusBadGateway { - time.Sleep(time.Duration(math.Pow(2, float64(i))) * delay) + time.Sleep(time.Duration(math.Pow(2, float64(i))) * delayInMillis) continue } return nil, err diff --git a/internal/wrappers/projects-http.go b/internal/wrappers/projects-http.go index ea311dfda..b3737e653 100644 --- a/internal/wrappers/projects-http.go +++ b/internal/wrappers/projects-http.go @@ -34,7 +34,7 @@ func (p *ProjectsHTTPWrapper) Create(model *Project) (*ProjectResponseModel, *Er fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPost, p.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -56,7 +56,7 @@ func (p *ProjectsHTTPWrapper) Update(projectID string, model *Project) error { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPut, fmt.Sprintf("%s/%s", p.path, projectID), bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return err } @@ -89,7 +89,7 @@ func (p *ProjectsHTTPWrapper) UpdateConfiguration(projectID string, configuratio fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodPatch, "api/configuration/project", params, bytes.NewBuffer(jsonBytes), clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, err } @@ -113,7 +113,7 @@ func (p *ProjectsHTTPWrapper) Get(params map[string]string) ( fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, p.path, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -154,7 +154,7 @@ func (p *ProjectsHTTPWrapper) GetByID(projectID string) ( fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, p.path+"/"+projectID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -201,7 +201,7 @@ func (p *ProjectsHTTPWrapper) GetBranchesByID(projectID string, params map[strin fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, p.path+request, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -239,7 +239,7 @@ func (p *ProjectsHTTPWrapper) Delete(projectID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodDelete, p.path+"/"+projectID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, err } @@ -260,7 +260,7 @@ func (p *ProjectsHTTPWrapper) Tags() ( fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, p.path+"/tags", http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } diff --git a/internal/wrappers/scans-http.go b/internal/wrappers/scans-http.go index 8c2067f66..b49d121a3 100644 --- a/internal/wrappers/scans-http.go +++ b/internal/wrappers/scans-http.go @@ -40,7 +40,7 @@ func (s *ScansHTTPWrapper) Create(model *Scan) (*ScanResponseModel, *ErrorModel, fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPost, s.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -58,7 +58,7 @@ func (s *ScansHTTPWrapper) Get(params map[string]string) (*ScansCollectionRespon fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, s.path, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -98,7 +98,7 @@ func (s *ScansHTTPWrapper) GetByID(scanID string) (*ScanResponseModel, *ErrorMod fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, s.path+"/"+scanID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -117,7 +117,7 @@ func (s *ScansHTTPWrapper) GetWorkflowByID(scanID string) ([]*ScanTaskResponseMo fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, path, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } @@ -162,7 +162,7 @@ func (s *ScansHTTPWrapper) Delete(scanID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodDelete, s.path+"/"+scanID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, err } @@ -186,7 +186,7 @@ func (s *ScansHTTPWrapper) Cancel(scanID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPatch, s.path+"/"+scanID, bytes.NewBuffer(b), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, err } @@ -204,7 +204,7 @@ func (s *ScansHTTPWrapper) Tags() (map[string][]string, *ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, s.path+"/tags", http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 3, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) if err != nil { return nil, nil, err } diff --git a/internal/wrappers/scans-http_test.go b/internal/wrappers/scans-http_test.go new file mode 100644 index 000000000..07da40a84 --- /dev/null +++ b/internal/wrappers/scans-http_test.go @@ -0,0 +1,61 @@ +//go:build !integration + +package wrappers + +import ( + "errors" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestRetryHTTPRequest_Success(t *testing.T) { + fn := func() (*http.Response, error) { + return &http.Response{StatusCode: http.StatusOK}, nil + } + + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +func TestRetryHTTPRequest_RetryOnBadGateway(t *testing.T) { + attempts := 0 + fn := func() (*http.Response, error) { + attempts++ + if attempts < 4 { + return &http.Response{StatusCode: http.StatusBadGateway}, nil + } + return &http.Response{StatusCode: http.StatusOK}, nil + } + + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, 4, attempts) +} + +func TestRetryHTTPRequest_FailAfterRetries(t *testing.T) { + fn := func() (*http.Response, error) { + return nil, errors.New("network error") + } + + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + assert.Error(t, err) + assert.Nil(t, resp) +} + +func TestRetryHTTPRequest_EndWithBadGateway(t *testing.T) { + fn := func() (*http.Response, error) { + return &http.Response{StatusCode: http.StatusBadGateway}, nil + } + + resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) +} From 9dd018670df56ceeb0eaee7cda441f085159069c Mon Sep 17 00:00:00 2001 From: galactica Date: Thu, 30 Jan 2025 15:29:21 +0200 Subject: [PATCH 3/6] Close resp and fix retry func --- internal/wrappers/client.go | 25 ++++++++++---------- internal/wrappers/scans-http_test.go | 34 ++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/internal/wrappers/client.go b/internal/wrappers/client.go index 021afb247..c99328adf 100644 --- a/internal/wrappers/client.go +++ b/internal/wrappers/client.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "io/ioutil" - "math" "net" "net/http" "net/http/httptrace" @@ -71,21 +70,23 @@ var cachedAccessToken string var cachedAccessTime time.Time var Domains = make(map[string]struct{}) -func retryHTTPRequest(fn func() (*http.Response, error), retries int, delayInMillis time.Duration) (*http.Response, error) { - var lastErr error - for i := 0; i < retries; i++ { - resp, err := fn() +func retryHTTPRequest(requestFunc func() (*http.Response, error), retries int, baseDelayInMillis time.Duration) (*http.Response, error) { + + var resp *http.Response + var err error + + for attempt := 0; attempt < retries; attempt++ { + resp, err = requestFunc() if err != nil { - lastErr = err - if resp != nil && resp.StatusCode == http.StatusBadGateway { - time.Sleep(time.Duration(math.Pow(2, float64(i))) * delayInMillis) - continue - } return nil, err } - return resp, nil + if resp.StatusCode != http.StatusBadGateway { + return resp, nil + } + _ = resp.Body.Close() + time.Sleep(baseDelayInMillis * (1 << attempt)) } - return nil, lastErr + return resp, nil } func setAgentName(req *http.Request) { diff --git a/internal/wrappers/scans-http_test.go b/internal/wrappers/scans-http_test.go index 07da40a84..176219dea 100644 --- a/internal/wrappers/scans-http_test.go +++ b/internal/wrappers/scans-http_test.go @@ -1,5 +1,3 @@ -//go:build !integration - package wrappers import ( @@ -11,9 +9,22 @@ import ( "github.com/stretchr/testify/assert" ) +type mockReadCloser struct{} + +func (m *mockReadCloser) Read(p []byte) (n int, err error) { + return 0, nil +} + +func (m *mockReadCloser) Close() error { + return nil +} + func TestRetryHTTPRequest_Success(t *testing.T) { fn := func() (*http.Response, error) { - return &http.Response{StatusCode: http.StatusOK}, nil + return &http.Response{ + StatusCode: http.StatusOK, + Body: &mockReadCloser{}, + }, nil } resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) @@ -27,9 +38,15 @@ func TestRetryHTTPRequest_RetryOnBadGateway(t *testing.T) { fn := func() (*http.Response, error) { attempts++ if attempts < 4 { - return &http.Response{StatusCode: http.StatusBadGateway}, nil + return &http.Response{ + StatusCode: http.StatusBadGateway, + Body: &mockReadCloser{}, + }, nil } - return &http.Response{StatusCode: http.StatusOK}, nil + return &http.Response{ + StatusCode: http.StatusOK, + Body: &mockReadCloser{}, + }, nil } resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) @@ -39,7 +56,7 @@ func TestRetryHTTPRequest_RetryOnBadGateway(t *testing.T) { assert.Equal(t, 4, attempts) } -func TestRetryHTTPRequest_FailAfterRetries(t *testing.T) { +func TestRetryHTTPRequest_Fail(t *testing.T) { fn := func() (*http.Response, error) { return nil, errors.New("network error") } @@ -51,7 +68,10 @@ func TestRetryHTTPRequest_FailAfterRetries(t *testing.T) { func TestRetryHTTPRequest_EndWithBadGateway(t *testing.T) { fn := func() (*http.Response, error) { - return &http.Response{StatusCode: http.StatusBadGateway}, nil + return &http.Response{ + StatusCode: http.StatusBadGateway, + Body: &mockReadCloser{}, + }, nil } resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) From da08bdccce05a35679265af84e20bc222c1ab670 Mon Sep 17 00:00:00 2001 From: galactica Date: Sun, 2 Feb 2025 12:44:58 +0200 Subject: [PATCH 4/6] Fix comment --- .../{scans-http_test.go => client_test.go} | 16 +++++++-------- internal/wrappers/projects-http.go | 20 +++++++++---------- internal/wrappers/scans-http.go | 16 ++++++++------- 3 files changed, 25 insertions(+), 27 deletions(-) rename internal/wrappers/{scans-http_test.go => client_test.go} (82%) diff --git a/internal/wrappers/scans-http_test.go b/internal/wrappers/client_test.go similarity index 82% rename from internal/wrappers/scans-http_test.go rename to internal/wrappers/client_test.go index 176219dea..66dcf79a7 100644 --- a/internal/wrappers/scans-http_test.go +++ b/internal/wrappers/client_test.go @@ -2,11 +2,9 @@ package wrappers import ( "errors" + "github.com/stretchr/testify/assert" "net/http" "testing" - "time" - - "github.com/stretchr/testify/assert" ) type mockReadCloser struct{} @@ -27,7 +25,7 @@ func TestRetryHTTPRequest_Success(t *testing.T) { }, nil } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) @@ -37,7 +35,7 @@ func TestRetryHTTPRequest_RetryOnBadGateway(t *testing.T) { attempts := 0 fn := func() (*http.Response, error) { attempts++ - if attempts < 4 { + if attempts < retryAttempts { return &http.Response{ StatusCode: http.StatusBadGateway, Body: &mockReadCloser{}, @@ -49,11 +47,11 @@ func TestRetryHTTPRequest_RetryOnBadGateway(t *testing.T) { }, nil } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) - assert.Equal(t, 4, attempts) + assert.Equal(t, retryAttempts, attempts) } func TestRetryHTTPRequest_Fail(t *testing.T) { @@ -61,7 +59,7 @@ func TestRetryHTTPRequest_Fail(t *testing.T) { return nil, errors.New("network error") } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) assert.Error(t, err) assert.Nil(t, resp) } @@ -74,7 +72,7 @@ func TestRetryHTTPRequest_EndWithBadGateway(t *testing.T) { }, nil } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, http.StatusBadGateway, resp.StatusCode) diff --git a/internal/wrappers/projects-http.go b/internal/wrappers/projects-http.go index b3737e653..7275ed5ce 100644 --- a/internal/wrappers/projects-http.go +++ b/internal/wrappers/projects-http.go @@ -4,11 +4,9 @@ import ( "bytes" "encoding/json" "fmt" - "net/http" - "time" - "github.com/pkg/errors" "github.com/spf13/viper" + "net/http" errorConstants "github.com/checkmarx/ast-cli/internal/constants/errors" commonParams "github.com/checkmarx/ast-cli/internal/params" @@ -34,7 +32,7 @@ func (p *ProjectsHTTPWrapper) Create(model *Project) (*ProjectResponseModel, *Er fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPost, p.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -56,7 +54,7 @@ func (p *ProjectsHTTPWrapper) Update(projectID string, model *Project) error { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPut, fmt.Sprintf("%s/%s", p.path, projectID), bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return err } @@ -89,7 +87,7 @@ func (p *ProjectsHTTPWrapper) UpdateConfiguration(projectID string, configuratio fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodPatch, "api/configuration/project", params, bytes.NewBuffer(jsonBytes), clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, err } @@ -113,7 +111,7 @@ func (p *ProjectsHTTPWrapper) Get(params map[string]string) ( fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, p.path, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -154,7 +152,7 @@ func (p *ProjectsHTTPWrapper) GetByID(projectID string) ( fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, p.path+"/"+projectID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -201,7 +199,7 @@ func (p *ProjectsHTTPWrapper) GetBranchesByID(projectID string, params map[strin fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, p.path+request, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -239,7 +237,7 @@ func (p *ProjectsHTTPWrapper) Delete(projectID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodDelete, p.path+"/"+projectID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, err } @@ -260,7 +258,7 @@ func (p *ProjectsHTTPWrapper) Tags() ( fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, p.path+"/tags", http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } diff --git a/internal/wrappers/scans-http.go b/internal/wrappers/scans-http.go index b49d121a3..f20f9ca57 100644 --- a/internal/wrappers/scans-http.go +++ b/internal/wrappers/scans-http.go @@ -16,6 +16,8 @@ const ( failedToParseGetAll = "Failed to parse list response" failedToParseTags = "Failed to parse tags response" failedToParseBranches = "Failed to parse branches response" + retryAttempts = 4 + retryDelay = 500 * time.Millisecond ) type ScansHTTPWrapper struct { @@ -40,7 +42,7 @@ func (s *ScansHTTPWrapper) Create(model *Scan) (*ScanResponseModel, *ErrorModel, fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPost, s.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -58,7 +60,7 @@ func (s *ScansHTTPWrapper) Get(params map[string]string) (*ScansCollectionRespon fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, s.path, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -98,7 +100,7 @@ func (s *ScansHTTPWrapper) GetByID(scanID string) (*ScanResponseModel, *ErrorMod fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, s.path+"/"+scanID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -117,7 +119,7 @@ func (s *ScansHTTPWrapper) GetWorkflowByID(scanID string) ([]*ScanTaskResponseMo fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, path, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } @@ -162,7 +164,7 @@ func (s *ScansHTTPWrapper) Delete(scanID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodDelete, s.path+"/"+scanID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, err } @@ -186,7 +188,7 @@ func (s *ScansHTTPWrapper) Cancel(scanID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPatch, s.path+"/"+scanID, bytes.NewBuffer(b), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, err } @@ -204,7 +206,7 @@ func (s *ScansHTTPWrapper) Tags() (map[string][]string, *ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, s.path+"/tags", http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, 4, 500*time.Millisecond) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) if err != nil { return nil, nil, err } From 7427c852bd2e5fcd415ff1e157e77354c879d56e Mon Sep 17 00:00:00 2001 From: galactica Date: Sun, 2 Feb 2025 14:08:28 +0200 Subject: [PATCH 5/6] Fix comment --- internal/wrappers/scans-http.go | 6 +----- internal/wrappers/wrapper-constants.go | 6 +++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/wrappers/scans-http.go b/internal/wrappers/scans-http.go index f20f9ca57..c16ec0a07 100644 --- a/internal/wrappers/scans-http.go +++ b/internal/wrappers/scans-http.go @@ -4,20 +4,16 @@ import ( "bytes" "encoding/json" "fmt" - "net/http" - "time" - commonParams "github.com/checkmarx/ast-cli/internal/params" "github.com/pkg/errors" "github.com/spf13/viper" + "net/http" ) const ( failedToParseGetAll = "Failed to parse list response" failedToParseTags = "Failed to parse tags response" failedToParseBranches = "Failed to parse branches response" - retryAttempts = 4 - retryDelay = 500 * time.Millisecond ) type ScansHTTPWrapper struct { diff --git a/internal/wrappers/wrapper-constants.go b/internal/wrappers/wrapper-constants.go index 242bebd6c..e0dbc9ec9 100644 --- a/internal/wrappers/wrapper-constants.go +++ b/internal/wrappers/wrapper-constants.go @@ -1,5 +1,9 @@ package wrappers +import "time" + const ( - limitValue = "10000" + limitValue = "10000" + retryAttempts = 4 + retryDelay = 500 * time.Millisecond ) From 421647041bad7055ac682d6ebc3c754fca6c839f Mon Sep 17 00:00:00 2001 From: galactica Date: Sun, 2 Feb 2025 14:28:13 +0200 Subject: [PATCH 6/6] Fix comment --- internal/wrappers/client.go | 4 ++-- internal/wrappers/client_test.go | 9 +++++---- .../{wrapper-constants.go => constants.go} | 4 +--- internal/wrappers/projects-http.go | 17 +++++++++-------- 4 files changed, 17 insertions(+), 17 deletions(-) rename internal/wrappers/{wrapper-constants.go => constants.go} (56%) diff --git a/internal/wrappers/client.go b/internal/wrappers/client.go index c99328adf..c1e7ade53 100644 --- a/internal/wrappers/client.go +++ b/internal/wrappers/client.go @@ -70,7 +70,7 @@ var cachedAccessToken string var cachedAccessTime time.Time var Domains = make(map[string]struct{}) -func retryHTTPRequest(requestFunc func() (*http.Response, error), retries int, baseDelayInMillis time.Duration) (*http.Response, error) { +func retryHTTPRequest(requestFunc func() (*http.Response, error), retries int, baseDelayInMilliSec time.Duration) (*http.Response, error) { var resp *http.Response var err error @@ -84,7 +84,7 @@ func retryHTTPRequest(requestFunc func() (*http.Response, error), retries int, b return resp, nil } _ = resp.Body.Close() - time.Sleep(baseDelayInMillis * (1 << attempt)) + time.Sleep(baseDelayInMilliSec * (1 << attempt)) } return resp, nil } diff --git a/internal/wrappers/client_test.go b/internal/wrappers/client_test.go index 66dcf79a7..f79577ce3 100644 --- a/internal/wrappers/client_test.go +++ b/internal/wrappers/client_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" "net/http" "testing" + "time" ) type mockReadCloser struct{} @@ -25,7 +26,7 @@ func TestRetryHTTPRequest_Success(t *testing.T) { }, nil } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) @@ -47,7 +48,7 @@ func TestRetryHTTPRequest_RetryOnBadGateway(t *testing.T) { }, nil } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) @@ -59,7 +60,7 @@ func TestRetryHTTPRequest_Fail(t *testing.T) { return nil, errors.New("network error") } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) assert.Error(t, err) assert.Nil(t, resp) } @@ -72,7 +73,7 @@ func TestRetryHTTPRequest_EndWithBadGateway(t *testing.T) { }, nil } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) assert.NoError(t, err) assert.NotNil(t, resp) assert.Equal(t, http.StatusBadGateway, resp.StatusCode) diff --git a/internal/wrappers/wrapper-constants.go b/internal/wrappers/constants.go similarity index 56% rename from internal/wrappers/wrapper-constants.go rename to internal/wrappers/constants.go index e0dbc9ec9..7c491e84a 100644 --- a/internal/wrappers/wrapper-constants.go +++ b/internal/wrappers/constants.go @@ -1,9 +1,7 @@ package wrappers -import "time" - const ( limitValue = "10000" retryAttempts = 4 - retryDelay = 500 * time.Millisecond + retryDelay = 500 ) diff --git a/internal/wrappers/projects-http.go b/internal/wrappers/projects-http.go index 7275ed5ce..556444b6f 100644 --- a/internal/wrappers/projects-http.go +++ b/internal/wrappers/projects-http.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/viper" "net/http" + "time" errorConstants "github.com/checkmarx/ast-cli/internal/constants/errors" commonParams "github.com/checkmarx/ast-cli/internal/params" @@ -32,7 +33,7 @@ func (p *ProjectsHTTPWrapper) Create(model *Project) (*ProjectResponseModel, *Er fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPost, p.path, bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, nil, err } @@ -54,7 +55,7 @@ func (p *ProjectsHTTPWrapper) Update(projectID string, model *Project) error { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodPut, fmt.Sprintf("%s/%s", p.path, projectID), bytes.NewBuffer(jsonBytes), true, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return err } @@ -87,7 +88,7 @@ func (p *ProjectsHTTPWrapper) UpdateConfiguration(projectID string, configuratio fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodPatch, "api/configuration/project", params, bytes.NewBuffer(jsonBytes), clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, err } @@ -111,7 +112,7 @@ func (p *ProjectsHTTPWrapper) Get(params map[string]string) ( fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, p.path, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, nil, err } @@ -152,7 +153,7 @@ func (p *ProjectsHTTPWrapper) GetByID(projectID string) ( fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, p.path+"/"+projectID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, nil, err } @@ -199,7 +200,7 @@ func (p *ProjectsHTTPWrapper) GetBranchesByID(projectID string, params map[strin fn := func() (*http.Response, error) { return SendHTTPRequestWithQueryParams(http.MethodGet, p.path+request, params, nil, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, nil, err } @@ -237,7 +238,7 @@ func (p *ProjectsHTTPWrapper) Delete(projectID string) (*ErrorModel, error) { fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodDelete, p.path+"/"+projectID, http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, err } @@ -258,7 +259,7 @@ func (p *ProjectsHTTPWrapper) Tags() ( fn := func() (*http.Response, error) { return SendHTTPRequest(http.MethodGet, p.path+"/tags", http.NoBody, true, clientTimeout) } - resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay) + resp, err := retryHTTPRequest(fn, retryAttempts, retryDelay*time.Millisecond) if err != nil { return nil, nil, err }