From ca269a998b23e0fd655f0b1137b99ddb39093223 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Wed, 27 Mar 2024 09:42:24 -0700 Subject: [PATCH 01/18] feat: add backoff/retry to azure (client side errors) and bheapi requests --- client/rest/client.go | 6 +++-- cmd/start.go | 60 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index 77b637f..95977db 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -254,8 +254,10 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { // Try the request if res, err = s.http.Do(req); err != nil { - // client error - return nil, err + fmt.Printf("ERR attempt=%d | req=%s | error=%v", retry+1, req.URL, err) + backoff := math.Pow(5, float64(retry+1)) + time.Sleep(time.Second * time.Duration(backoff)) + continue } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { // Error response code handling // See official Retry guidance (https://learn.microsoft.com/en-us/azure/architecture/best-practices/retry-service-specific#retry-usage-guidance) diff --git a/cmd/start.go b/cmd/start.go index 3ea2afd..024f37b 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -256,19 +256,59 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch // TODO: create/use a proper bloodhound client func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { - if res, err := bheClient.Do(req); err != nil { - return nil, fmt.Errorf("failed to request %v: %w", req.URL, err) - } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { - var body json.RawMessage - defer res.Body.Close() - if err := json.NewDecoder(res.Body).Decode(&body); err != nil { - return nil, fmt.Errorf("received unexpected response code from %v: %s; failure reading response body", req.URL, res.Status) + var ( + body []byte + res *http.Response + err error + maxRetries = 3 + ) + + // copy the bytes in case we need to retry the request + if req.Body != nil { + if body, err = io.ReadAll(req.Body); err != nil { + return nil, err + } + if body != nil { + req.Body = io.NopCloser(bytes.NewBuffer(body)) + } + } + + for retry := 0; retry < maxRetries; retry++ { + // Reusing http.Request requires rewinding the request body + // back to a working state + if body != nil && retry > 0 { + req.Body = io.NopCloser(bytes.NewBuffer(body)) + } + + if res, err = bheClient.Do(req); err != nil { + // should we only be checking for a failed connection?? + // Client error, try again to attempt avoiding transient errors + fmt.Printf("ERR attempt=%d | req=%s | ERR=%v\n", retry+1, req.URL, err) + backoff := math.Pow(5, float64(retry+1)) + time.Sleep(time.Second * time.Duration(backoff)) + continue + } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { + if res.StatusCode >= http.StatusInternalServerError { + // Internal server error, backoff and try again. + fmt.Printf("ERR attempt=%d | req=%s | ERR=%v\n", retry+1, req.URL, err) + backoff := math.Pow(5, float64(retry+1)) + time.Sleep(time.Second * time.Duration(backoff)) + continue + } + // bad request we do not need to retry + var body json.RawMessage + defer res.Body.Close() + if err := json.NewDecoder(res.Body).Decode(&body); err != nil { + return nil, fmt.Errorf("received unexpected response code from %v: %s; failure reading response body", req.URL, res.Status) + } else { + return nil, fmt.Errorf("received unexpected response code from %v: %s %s", req.URL, res.Status, body) + } } else { - return nil, fmt.Errorf("received unexpected response code from %v: %s %s", req.URL, res.Status, body) + return res, nil } - } else { - return res, nil } + + return nil, fmt.Errorf("unable to complete request | url=%s | attempts=%d | ERR=%w", req.URL, maxRetries, err) } type basicResponse[T any] struct { From 583abd18a11f5cc011080a44bbda7172815c1269 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Fri, 29 Mar 2024 14:07:50 -0700 Subject: [PATCH 02/18] feat: retry 502 and azure force closed connections --- client/rest/client.go | 14 ++++++++++---- client/rest/client_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 client/rest/client_test.go diff --git a/client/rest/client.go b/client/rest/client.go index 95977db..d880366 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -29,6 +29,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "sync" "time" @@ -254,10 +255,15 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { // Try the request if res, err = s.http.Do(req); err != nil { - fmt.Printf("ERR attempt=%d | req=%s | error=%v", retry+1, req.URL, err) - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) - continue + // TODO: maybe add a test case for this? + if strings.Contains(err.Error(), "An existing connection was forcibly closed by the remote host.") { + // TODO: probably should use a proper logger here? + fmt.Printf("ERR attempt=%d | req=%s | error=%v", retry+1, req.URL, err) + backoff := math.Pow(5, float64(retry+1)) + time.Sleep(time.Second * time.Duration(backoff)) + continue + } + return nil, err } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { // Error response code handling // See official Retry guidance (https://learn.microsoft.com/en-us/azure/architecture/best-practices/retry-service-specific#retry-usage-guidance) diff --git a/client/rest/client_test.go b/client/rest/client_test.go new file mode 100644 index 0000000..e263ee6 --- /dev/null +++ b/client/rest/client_test.go @@ -0,0 +1,39 @@ +// Copyright (C) 2024 Specter Ops, Inc. +// +// This file is part of AzureHound. +// +// AzureHound is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// AzureHound is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package rest + +import ( + "testing" +) + +func TestClosedConnection(t *testing.T) { + // var s *httptest.Server + // var h http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) { + // fmt.Println("closing client connections") + // s.CloseClientConnections() + // } + // s = httptest.NewServer(h) + // defer s.Close() + + // res, err := RestClient.Get(gomock.Any(), s.URL) + // if err == nil { + // t.Fatalf("Something aint right, err should be nil %v", err) + // } + + // fmt.Printf("res:%v,err:%v", res, err) +} From 4de18898f1edc22e1da3140ad8546ec98ec431a0 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Fri, 29 Mar 2024 14:08:40 -0700 Subject: [PATCH 03/18] feat: retry bheapi 502 and force closed connections --- cmd/start.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/cmd/start.go b/cmd/start.go index 024f37b..7292465 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -32,6 +32,7 @@ import ( "os/signal" "runtime" "sort" + "strings" "sync" "sync/atomic" "time" @@ -222,9 +223,13 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch if response, err := bheClient.Do(req); err != nil { log.Error(err, unrecoverableErrMsg) return true - } else if response.StatusCode == http.StatusGatewayTimeout || response.StatusCode == http.StatusServiceUnavailable { + } else if response.StatusCode == http.StatusGatewayTimeout || response.StatusCode == http.StatusServiceUnavailable || response.StatusCode == http.StatusBadGateway { + serverError := fmt.Errorf("received server error %d while requesting %v", response.StatusCode, endpoint) + log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) + backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) + if retry == maxRetries-1 { log.Error(ErrExceededRetryLimit, "") hasErrors = true @@ -281,16 +286,21 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { } if res, err = bheClient.Do(req); err != nil { - // should we only be checking for a failed connection?? - // Client error, try again to attempt avoiding transient errors - fmt.Printf("ERR attempt=%d | req=%s | ERR=%v\n", retry+1, req.URL, err) - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) - continue + if strings.Contains(err.Error(), "An existing connection was forcibly closed by the remote host.") { + // try again on force closed connections + log.Error(err, "server error while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) + backoff := math.Pow(5, float64(retry+1)) + time.Sleep(time.Second * time.Duration(backoff)) + continue + } + // normal client error, dont attempt again + return nil, err } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { if res.StatusCode >= http.StatusInternalServerError { // Internal server error, backoff and try again. - fmt.Printf("ERR attempt=%d | req=%s | ERR=%v\n", retry+1, req.URL, err) + serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) + log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) + backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) continue From dd043ac5d109ffbf0424eebe24a6c45f2fe521fc Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Fri, 29 Mar 2024 14:57:35 -0700 Subject: [PATCH 04/18] style: change closed connection log message --- client/rest/client.go | 4 ++-- cmd/start.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index d880366..e79d548 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -257,8 +257,8 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { if res, err = s.http.Do(req); err != nil { // TODO: maybe add a test case for this? if strings.Contains(err.Error(), "An existing connection was forcibly closed by the remote host.") { - // TODO: probably should use a proper logger here? - fmt.Printf("ERR attempt=%d | req=%s | error=%v", retry+1, req.URL, err) + + fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) continue diff --git a/cmd/start.go b/cmd/start.go index 7292465..4cc4e1e 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -288,7 +288,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res, err = bheClient.Do(req); err != nil { if strings.Contains(err.Error(), "An existing connection was forcibly closed by the remote host.") { // try again on force closed connections - log.Error(err, "server error while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) continue From 261c045bf27c12dfd0ea68b00dec51695d590850 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Mon, 1 Apr 2024 11:46:41 -0700 Subject: [PATCH 05/18] test: TestClosedConnection --- client/rest/client.go | 7 ++-- client/rest/client_test.go | 65 ++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index e79d548..43e22fa 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -255,10 +255,9 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { // Try the request if res, err = s.http.Do(req); err != nil { - // TODO: maybe add a test case for this? - if strings.Contains(err.Error(), "An existing connection was forcibly closed by the remote host.") { - - fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) + closedConnectionMsg := "An existing connection was forcibly closed by the remote host." + if strings.Contains(err.Error(), closedConnectionMsg) || strings.HasSuffix(err.Error(), ": EOF") { + fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) continue diff --git a/client/rest/client_test.go b/client/rest/client_test.go index e263ee6..ca8e4c5 100644 --- a/client/rest/client_test.go +++ b/client/rest/client_test.go @@ -18,22 +18,59 @@ package rest import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/bloodhoundad/azurehound/v2/client/config" ) func TestClosedConnection(t *testing.T) { - // var s *httptest.Server - // var h http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) { - // fmt.Println("closing client connections") - // s.CloseClientConnections() - // } - // s = httptest.NewServer(h) - // defer s.Close() - - // res, err := RestClient.Get(gomock.Any(), s.URL) - // if err == nil { - // t.Fatalf("Something aint right, err should be nil %v", err) - // } - - // fmt.Printf("res:%v,err:%v", res, err) + attempt := 0 + var testServer *httptest.Server + var mockHandler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) { + attempt++ + testServer.CloseClientConnections() + } + + testServer = httptest.NewServer(mockHandler) + defer testServer.Close() + + defaultConfig := config.Config{ + Username: "azurehound", + Password: "we_collect", + Authority: testServer.URL, + } + + if client, err := NewRestClient(testServer.URL, defaultConfig); err != nil { + t.Fatalf("error initializing rest client %v", err) + } else { + if req, err := http.NewRequest(http.MethodGet, testServer.URL, nil); err != nil { + t.Fatalf("error creating request %v", err) + } else { + didSucceed := false + + // make request in separate goroutine so we can end it early after the first retry + go func() { + // end request on the second attempt after a closed connection + if res, err := client.Send(req); err != nil { + fmt.Println(err) + } else if res.Status == "200" { + didSucceed = true + } + }() + + for attempt < 3 { + if attempt > 1 { + return + } + } + + if didSucceed { + t.Fatalf("expected an attempted retry but the request succeeded") + } + } + } } From 72f06517f95cb1d2caa0d0bc55744cdcf85173fb Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Mon, 1 Apr 2024 17:19:38 -0700 Subject: [PATCH 06/18] refactor: utils for err check and backoff --- client/rest/client.go | 7 ++----- client/rest/utils.go | 14 ++++++++++++++ cmd/start.go | 13 +++++++++---- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index 43e22fa..d666d10 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -29,7 +29,6 @@ import ( "net/http" "net/url" "strconv" - "strings" "sync" "time" @@ -255,11 +254,9 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { // Try the request if res, err = s.http.Do(req); err != nil { - closedConnectionMsg := "An existing connection was forcibly closed by the remote host." - if strings.Contains(err.Error(), closedConnectionMsg) || strings.HasSuffix(err.Error(), ": EOF") { + if IsClosedConnectionErr(err) { fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) + ExponentialBackoff(retry, maxRetries) continue } return nil, err diff --git a/client/rest/utils.go b/client/rest/utils.go index b048e7b..941a688 100644 --- a/client/rest/utils.go +++ b/client/rest/utils.go @@ -25,6 +25,7 @@ import ( "encoding/pem" "fmt" "io" + "math" "strings" "time" @@ -120,3 +121,16 @@ func x5t(certificate string) (string, error) { return base64.StdEncoding.EncodeToString(checksum[:]), nil } } + +var ClosedConnectionMsg = "An existing connection was forcibly closed by the remote host." + +func IsClosedConnectionErr(err error) bool { + closedFromClient := strings.Contains(err.Error(), ClosedConnectionMsg) + closedFromTestCase := strings.HasSuffix(err.Error(), ": EOF") + return closedFromClient || closedFromTestCase +} + +func ExponentialBackoff(retry int, maxRetries int) { + backoff := math.Pow(5, float64(retry+1)) + time.Sleep(time.Second * time.Duration(backoff)) +} diff --git a/cmd/start.go b/cmd/start.go index 4cc4e1e..3799ef2 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -221,6 +221,12 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch for retry := 0; retry < maxRetries; retry++ { // No retries on regular err cases, only on HTTP 504 Gateway Timeout and HTTP 503 Service Unavailable if response, err := bheClient.Do(req); err != nil { + if rest.IsClosedConnectionErr(err) { + // try again on force closed connection + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) + rest.ExponentialBackoff(retry, maxRetries) + continue + } log.Error(err, unrecoverableErrMsg) return true } else if response.StatusCode == http.StatusGatewayTimeout || response.StatusCode == http.StatusServiceUnavailable || response.StatusCode == http.StatusBadGateway { @@ -286,11 +292,10 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { } if res, err = bheClient.Do(req); err != nil { - if strings.Contains(err.Error(), "An existing connection was forcibly closed by the remote host.") { + if strings.Contains(err.Error(), rest.ClosedConnectionMsg) || strings.HasSuffix(err.Error(), ": EOF") { // try again on force closed connections - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) + rest.ExponentialBackoff(retry, maxRetries) continue } // normal client error, dont attempt again From 1403e2c2393ac397f4f9f33260d3a03bde9a7f93 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Mon, 1 Apr 2024 17:22:22 -0700 Subject: [PATCH 07/18] refactor: replcae more of the backoff method --- client/rest/client.go | 4 +--- cmd/start.go | 7 ++----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index d666d10..2093961 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -25,7 +25,6 @@ import ( "encoding/json" "fmt" "io" - "math" "net/http" "net/url" "strconv" @@ -274,8 +273,7 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { } } else if res.StatusCode >= http.StatusInternalServerError { // Wait the time calculated by the 5 second exponential backoff - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) + ExponentialBackoff(retry, maxRetries) continue } else { // Not a status code that warrants a retry diff --git a/cmd/start.go b/cmd/start.go index 3799ef2..c832a46 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -25,7 +25,6 @@ import ( "errors" "fmt" "io" - "math" "net/http" "net/url" "os" @@ -233,8 +232,7 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch serverError := fmt.Errorf("received server error %d while requesting %v", response.StatusCode, endpoint) log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) + rest.ExponentialBackoff(retry, maxRetries) if retry == maxRetries-1 { log.Error(ErrExceededRetryLimit, "") @@ -306,8 +304,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) - backoff := math.Pow(5, float64(retry+1)) - time.Sleep(time.Second * time.Duration(backoff)) + rest.ExponentialBackoff(retry, maxRetries) continue } // bad request we do not need to retry From a5afd3241dbb494315de7b29b5189e14e0581b4c Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 13:51:41 -0700 Subject: [PATCH 08/18] fix: requestCompleted logic and comments --- client/rest/client_test.go | 24 ++++++++++-------------- client/rest/utils.go | 2 ++ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/client/rest/client_test.go b/client/rest/client_test.go index ca8e4c5..85dbb3b 100644 --- a/client/rest/client_test.go +++ b/client/rest/client_test.go @@ -18,7 +18,6 @@ package rest import ( - "fmt" "net/http" "net/http/httptest" @@ -50,26 +49,23 @@ func TestClosedConnection(t *testing.T) { if req, err := http.NewRequest(http.MethodGet, testServer.URL, nil); err != nil { t.Fatalf("error creating request %v", err) } else { - didSucceed := false + requestCompleted := false - // make request in separate goroutine so we can end it early after the first retry + // make request in separate goroutine so its not blocking after we validated the retry go func() { - // end request on the second attempt after a closed connection - if res, err := client.Send(req); err != nil { - fmt.Println(err) - } else if res.Status == "200" { - didSucceed = true - } + client.Send(req) + requestCompleted = true }() - for attempt < 3 { - if attempt > 1 { - return + // block until attempt is > 2 or request succeeds + for attempt <= 2 { + if attempt > 1 || requestCompleted { + break } } - if didSucceed { - t.Fatalf("expected an attempted retry but the request succeeded") + if requestCompleted { + t.Fatalf("expected an attempted retry but the request completed") } } } diff --git a/client/rest/utils.go b/client/rest/utils.go index 941a688..e183156 100644 --- a/client/rest/utils.go +++ b/client/rest/utils.go @@ -126,6 +126,8 @@ var ClosedConnectionMsg = "An existing connection was forcibly closed by the rem func IsClosedConnectionErr(err error) bool { closedFromClient := strings.Contains(err.Error(), ClosedConnectionMsg) + // Mocking http.Do would require a larger refactor, + // so closedFromTestCase is used for testing only. closedFromTestCase := strings.HasSuffix(err.Error(), ": EOF") return closedFromClient || closedFromTestCase } From f5003ecfe69dd178219653e5fa177fa63e160490 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 14:41:57 -0700 Subject: [PATCH 09/18] refactor: remove NewRequest and improve comments --- client/rest/client_test.go | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/client/rest/client_test.go b/client/rest/client_test.go index 85dbb3b..3525aef 100644 --- a/client/rest/client_test.go +++ b/client/rest/client_test.go @@ -27,8 +27,8 @@ import ( ) func TestClosedConnection(t *testing.T) { - attempt := 0 var testServer *httptest.Server + attempt := 0 var mockHandler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) { attempt++ testServer.CloseClientConnections() @@ -46,27 +46,24 @@ func TestClosedConnection(t *testing.T) { if client, err := NewRestClient(testServer.URL, defaultConfig); err != nil { t.Fatalf("error initializing rest client %v", err) } else { - if req, err := http.NewRequest(http.MethodGet, testServer.URL, nil); err != nil { - t.Fatalf("error creating request %v", err) - } else { - requestCompleted := false + requestCompleted := false - // make request in separate goroutine so its not blocking after we validated the retry - go func() { - client.Send(req) - requestCompleted = true - }() + // make request in separate goroutine so its not blocking after we validated the retry + go func() { + client.Authenticate() // Authenticate()because it uses the internal client.send method. + // the above request should block this from running, however if it does then the test fails. + requestCompleted = true + }() - // block until attempt is > 2 or request succeeds - for attempt <= 2 { - if attempt > 1 || requestCompleted { - break - } + // block until attempt is > 2 or request succeeds + for attempt <= 2 { + if attempt > 1 || requestCompleted { + break } + } - if requestCompleted { - t.Fatalf("expected an attempted retry but the request completed") - } + if requestCompleted { + t.Fatalf("expected an attempted retry but the request completed") } } } From 970c6c70fff3b5e7e12b6461f0de904a2d6916d5 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 15:06:23 -0700 Subject: [PATCH 10/18] refactor: make CopyBody exported from rest package --- client/rest/client.go | 16 +------- client/rest/client_test.go | 4 +- client/rest/utils.go | 21 ++++++++-- cmd/start.go | 83 +++++++++++++++++--------------------- 4 files changed, 59 insertions(+), 65 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index 2093961..7963e8b 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -218,23 +218,9 @@ func (s *restClient) Send(req *http.Request) (*http.Response, error) { return s.send(req) } -func copyBody(req *http.Request) ([]byte, error) { - var ( - body []byte - err error - ) - if req.Body != nil { - body, err = io.ReadAll(req.Body) - if body != nil { - req.Body = io.NopCloser(bytes.NewBuffer(body)) - } - } - return body, err -} - func (s *restClient) send(req *http.Request) (*http.Response, error) { // copy the bytes in case we need to retry the request - if body, err := copyBody(req); err != nil { + if body, err := CopyBody(req); err != nil { return nil, err } else { var ( diff --git a/client/rest/client_test.go b/client/rest/client_test.go index 3525aef..c452c0e 100644 --- a/client/rest/client_test.go +++ b/client/rest/client_test.go @@ -50,8 +50,8 @@ func TestClosedConnection(t *testing.T) { // make request in separate goroutine so its not blocking after we validated the retry go func() { - client.Authenticate() // Authenticate()because it uses the internal client.send method. - // the above request should block this from running, however if it does then the test fails. + client.Authenticate() // Authenticate() because it uses the internal client.send method. + // the above request should block the request from completing, however if it does then the test fails. requestCompleted = true }() diff --git a/client/rest/utils.go b/client/rest/utils.go index e183156..6247ea5 100644 --- a/client/rest/utils.go +++ b/client/rest/utils.go @@ -18,6 +18,7 @@ package rest import ( + "bytes" "crypto/sha1" "crypto/x509" "encoding/base64" @@ -26,6 +27,7 @@ import ( "fmt" "io" "math" + "net/http" "strings" "time" @@ -122,10 +124,9 @@ func x5t(certificate string) (string, error) { } } -var ClosedConnectionMsg = "An existing connection was forcibly closed by the remote host." - func IsClosedConnectionErr(err error) bool { - closedFromClient := strings.Contains(err.Error(), ClosedConnectionMsg) + var closedConnectionMsg = "An existing connection was forcibly closed by the remote host." + closedFromClient := strings.Contains(err.Error(), closedConnectionMsg) // Mocking http.Do would require a larger refactor, // so closedFromTestCase is used for testing only. closedFromTestCase := strings.HasSuffix(err.Error(), ": EOF") @@ -136,3 +137,17 @@ func ExponentialBackoff(retry int, maxRetries int) { backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) } + +func CopyBody(req *http.Request) ([]byte, error) { + var ( + body []byte + err error + ) + if req.Body != nil { + body, err = io.ReadAll(req.Body) + if body != nil { + req.Body = io.NopCloser(bytes.NewBuffer(body)) + } + } + return body, err +} diff --git a/cmd/start.go b/cmd/start.go index c832a46..9b263d3 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -31,7 +31,6 @@ import ( "os/signal" "runtime" "sort" - "strings" "sync" "sync/atomic" "time" @@ -229,8 +228,8 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch log.Error(err, unrecoverableErrMsg) return true } else if response.StatusCode == http.StatusGatewayTimeout || response.StatusCode == http.StatusServiceUnavailable || response.StatusCode == http.StatusBadGateway { - serverError := fmt.Errorf("received server error %d while requesting %v", response.StatusCode, endpoint) - log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) + serverError := fmt.Errorf("received server error %d while requesting %v;", response.StatusCode, endpoint) + log.Error(serverError, "attempt %d/%d; trying again", retry+1, maxRetries) rest.ExponentialBackoff(retry, maxRetries) @@ -266,61 +265,55 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch // TODO: create/use a proper bloodhound client func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { var ( - body []byte res *http.Response err error maxRetries = 3 ) // copy the bytes in case we need to retry the request - if req.Body != nil { - if body, err = io.ReadAll(req.Body); err != nil { - return nil, err - } - if body != nil { - req.Body = io.NopCloser(bytes.NewBuffer(body)) - } - } + if body, err := rest.CopyBody(req); err != nil { + return nil, err + } else { + for retry := 0; retry < maxRetries; retry++ { + // Reusing http.Request requires rewinding the request body + // back to a working state + if body != nil && retry > 0 { + req.Body = io.NopCloser(bytes.NewBuffer(body)) + } - for retry := 0; retry < maxRetries; retry++ { - // Reusing http.Request requires rewinding the request body - // back to a working state - if body != nil && retry > 0 { - req.Body = io.NopCloser(bytes.NewBuffer(body)) - } + if res, err = bheClient.Do(req); err != nil { + if rest.IsClosedConnectionErr(err) { + // try again on force closed connections + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) + rest.ExponentialBackoff(retry, maxRetries) + continue + } + // normal client error, dont attempt again + return nil, err + } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { + if res.StatusCode >= http.StatusInternalServerError { + // Internal server error, backoff and try again. + serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) + log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) - if res, err = bheClient.Do(req); err != nil { - if strings.Contains(err.Error(), rest.ClosedConnectionMsg) || strings.HasSuffix(err.Error(), ": EOF") { - // try again on force closed connections - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) - rest.ExponentialBackoff(retry, maxRetries) - continue - } - // normal client error, dont attempt again - return nil, err - } else if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest { - if res.StatusCode >= http.StatusInternalServerError { - // Internal server error, backoff and try again. - serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) - log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) - - rest.ExponentialBackoff(retry, maxRetries) - continue - } - // bad request we do not need to retry - var body json.RawMessage - defer res.Body.Close() - if err := json.NewDecoder(res.Body).Decode(&body); err != nil { - return nil, fmt.Errorf("received unexpected response code from %v: %s; failure reading response body", req.URL, res.Status) + rest.ExponentialBackoff(retry, maxRetries) + continue + } + // bad request we do not need to retry + var body json.RawMessage + defer res.Body.Close() + if err := json.NewDecoder(res.Body).Decode(&body); err != nil { + return nil, fmt.Errorf("received unexpected response code from %v: %s; failure reading response body", req.URL, res.Status) + } else { + return nil, fmt.Errorf("received unexpected response code from %v: %s %s", req.URL, res.Status, body) + } } else { - return nil, fmt.Errorf("received unexpected response code from %v: %s %s", req.URL, res.Status, body) + return res, nil } - } else { - return res, nil } } - return nil, fmt.Errorf("unable to complete request | url=%s | attempts=%d | ERR=%w", req.URL, maxRetries, err) + return nil, fmt.Errorf("unable to complete request to url=%s; attempts=%d; ERR=%w", req.URL, maxRetries, err) } type basicResponse[T any] struct { From e81ef5d4f434c8c542afba708245e225f8ddf5dd Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 15:08:37 -0700 Subject: [PATCH 11/18] refactor: start doesnt need the err state --- cmd/start.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/start.go b/cmd/start.go index 9b263d3..fd21c62 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -266,7 +266,6 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { var ( res *http.Response - err error maxRetries = 3 ) @@ -313,7 +312,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { } } - return nil, fmt.Errorf("unable to complete request to url=%s; attempts=%d; ERR=%w", req.URL, maxRetries, err) + return nil, fmt.Errorf("unable to complete request to url=%s; attempts=%d;", req.URL, maxRetries) } type basicResponse[T any] struct { From ed99282403acec4eafa4a2ce0b3c62d6d39d2129 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 15:34:05 -0700 Subject: [PATCH 12/18] fix: ExponentialBackoff only needs retry param --- client/rest/client.go | 4 ++-- client/rest/utils.go | 2 +- cmd/start.go | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index 7963e8b..919edc4 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -241,7 +241,7 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { if res, err = s.http.Do(req); err != nil { if IsClosedConnectionErr(err) { fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) - ExponentialBackoff(retry, maxRetries) + ExponentialBackoff(retry) continue } return nil, err @@ -259,7 +259,7 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { } } else if res.StatusCode >= http.StatusInternalServerError { // Wait the time calculated by the 5 second exponential backoff - ExponentialBackoff(retry, maxRetries) + ExponentialBackoff(retry) continue } else { // Not a status code that warrants a retry diff --git a/client/rest/utils.go b/client/rest/utils.go index 6247ea5..1d123d4 100644 --- a/client/rest/utils.go +++ b/client/rest/utils.go @@ -133,7 +133,7 @@ func IsClosedConnectionErr(err error) bool { return closedFromClient || closedFromTestCase } -func ExponentialBackoff(retry int, maxRetries int) { +func ExponentialBackoff(retry int) { backoff := math.Pow(5, float64(retry+1)) time.Sleep(time.Second * time.Duration(backoff)) } diff --git a/cmd/start.go b/cmd/start.go index fd21c62..c6db844 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -222,16 +222,16 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch if rest.IsClosedConnectionErr(err) { // try again on force closed connection log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) - rest.ExponentialBackoff(retry, maxRetries) + rest.ExponentialBackoff(retry) continue } log.Error(err, unrecoverableErrMsg) return true } else if response.StatusCode == http.StatusGatewayTimeout || response.StatusCode == http.StatusServiceUnavailable || response.StatusCode == http.StatusBadGateway { serverError := fmt.Errorf("received server error %d while requesting %v;", response.StatusCode, endpoint) - log.Error(serverError, "attempt %d/%d; trying again", retry+1, maxRetries) + log.Error(serverError, "attempt %d/%d; trying again...", retry+1, maxRetries) - rest.ExponentialBackoff(retry, maxRetries) + rest.ExponentialBackoff(retry) if retry == maxRetries-1 { log.Error(ErrExceededRetryLimit, "") @@ -284,7 +284,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if rest.IsClosedConnectionErr(err) { // try again on force closed connections log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) - rest.ExponentialBackoff(retry, maxRetries) + rest.ExponentialBackoff(retry) continue } // normal client error, dont attempt again @@ -293,9 +293,9 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res.StatusCode >= http.StatusInternalServerError { // Internal server error, backoff and try again. serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) - log.Error(serverError, "attempt %d/%d", retry+1, maxRetries) + log.Error(serverError, "attempt %d/%d; trying again...", retry+1, maxRetries) - rest.ExponentialBackoff(retry, maxRetries) + rest.ExponentialBackoff(retry) continue } // bad request we do not need to retry From 87a8d64a07141a8511f89f0d5dfcc0d34b984816 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 15:40:15 -0700 Subject: [PATCH 13/18] fix: comment and logs --- client/rest/client_test.go | 2 +- client/rest/utils.go | 3 +-- cmd/start.go | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/client/rest/client_test.go b/client/rest/client_test.go index c452c0e..4be3273 100644 --- a/client/rest/client_test.go +++ b/client/rest/client_test.go @@ -51,7 +51,7 @@ func TestClosedConnection(t *testing.T) { // make request in separate goroutine so its not blocking after we validated the retry go func() { client.Authenticate() // Authenticate() because it uses the internal client.send method. - // the above request should block the request from completing, however if it does then the test fails. + // CloseClientConnections should block the request from completing, however if it completes then the test fails. requestCompleted = true }() diff --git a/client/rest/utils.go b/client/rest/utils.go index 1d123d4..ed3b85e 100644 --- a/client/rest/utils.go +++ b/client/rest/utils.go @@ -127,8 +127,7 @@ func x5t(certificate string) (string, error) { func IsClosedConnectionErr(err error) bool { var closedConnectionMsg = "An existing connection was forcibly closed by the remote host." closedFromClient := strings.Contains(err.Error(), closedConnectionMsg) - // Mocking http.Do would require a larger refactor, - // so closedFromTestCase is used for testing only. + // Mocking http.Do would require a larger refactor, so closedFromTestCase is used to cover testing only. closedFromTestCase := strings.HasSuffix(err.Error(), ": EOF") return closedFromClient || closedFromTestCase } diff --git a/cmd/start.go b/cmd/start.go index c6db844..b6546e8 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -221,7 +221,7 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch if response, err := bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connection - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) continue } @@ -283,7 +283,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res, err = bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connections - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) continue } From 10c4fc6c414bcdec9950cc0a0069629eeecfe379 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 15:40:45 -0700 Subject: [PATCH 14/18] feat: remove prefer timeout --- cmd/start.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/start.go b/cmd/start.go index b6546e8..e7609e7 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -214,7 +214,6 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch } else { req.Header.Set("User-Agent", constants.UserAgent()) req.Header.Set("Accept", "application/json") - req.Header.Set("Prefer", "wait=60") req.Header.Set("Content-Encoding", "gzip") for retry := 0; retry < maxRetries; retry++ { // No retries on regular err cases, only on HTTP 504 Gateway Timeout and HTTP 503 Service Unavailable From 2c4e34dc40e0054a4e50ccd65f323d2b09bbb45c Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 15:49:34 -0700 Subject: [PATCH 15/18] feat: dont stop ingest if there is a closed connection --- cmd/start.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/start.go b/cmd/start.go index e7609e7..9133e90 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -222,6 +222,12 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch // try again on force closed connection log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) + + if retry == maxRetries-1 { + log.Error(ErrExceededRetryLimit, "") + hasErrors = true + } + continue } log.Error(err, unrecoverableErrMsg) From 3f3dabe41a6b3ce01f249ed8fb73d7b6daec63e9 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 16:01:15 -0700 Subject: [PATCH 16/18] fix: log.Error doesnt need a new line character --- client/rest/client.go | 2 +- cmd/start.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index 919edc4..8b110f9 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -240,7 +240,7 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { // Try the request if res, err = s.http.Do(req); err != nil { if IsClosedConnectionErr(err) { - fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) + fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) ExponentialBackoff(retry) continue } diff --git a/cmd/start.go b/cmd/start.go index 9133e90..4c6bd2a 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -220,7 +220,7 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch if response, err := bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connection - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) if retry == maxRetries-1 { @@ -288,7 +288,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res, err = bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connections - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) continue } From 976964f21e275aecd46ed25612c9bbd3ed4051e0 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 2 Apr 2024 16:05:53 -0700 Subject: [PATCH 17/18] fix: error messages arent supposed to end in puncuation I guess --- client/rest/client.go | 2 +- cmd/start.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/rest/client.go b/client/rest/client.go index 8b110f9..919edc4 100644 --- a/client/rest/client.go +++ b/client/rest/client.go @@ -240,7 +240,7 @@ func (s *restClient) send(req *http.Request) (*http.Response, error) { // Try the request if res, err = s.http.Do(req); err != nil { if IsClosedConnectionErr(err) { - fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again...\n", req.URL, retry+1, maxRetries) + fmt.Printf("remote host force closed connection while requesting %s; attempt %d/%d; trying again\n", req.URL, retry+1, maxRetries) ExponentialBackoff(retry) continue } diff --git a/cmd/start.go b/cmd/start.go index 4c6bd2a..8693140 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -220,7 +220,7 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch if response, err := bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connection - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) if retry == maxRetries-1 { @@ -233,8 +233,8 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch log.Error(err, unrecoverableErrMsg) return true } else if response.StatusCode == http.StatusGatewayTimeout || response.StatusCode == http.StatusServiceUnavailable || response.StatusCode == http.StatusBadGateway { - serverError := fmt.Errorf("received server error %d while requesting %v;", response.StatusCode, endpoint) - log.Error(serverError, "attempt %d/%d; trying again...", retry+1, maxRetries) + serverError := fmt.Errorf("received server error %d while requesting %v; attempt %d/%d; trying again", response.StatusCode, endpoint, retry+1, maxRetries) + log.Error(serverError, "") rest.ExponentialBackoff(retry) @@ -288,7 +288,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res, err = bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connections - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again...", req.URL, retry+1, maxRetries) + log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) rest.ExponentialBackoff(retry) continue } @@ -298,7 +298,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res.StatusCode >= http.StatusInternalServerError { // Internal server error, backoff and try again. serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) - log.Error(serverError, "attempt %d/%d; trying again...", retry+1, maxRetries) + log.Error(serverError, "attempt %d/%d; trying again", retry+1, maxRetries) rest.ExponentialBackoff(retry) continue From 05619cfc8ff1089d2777b760bc3ca0518506aa34 Mon Sep 17 00:00:00 2001 From: Ben Waples Date: Tue, 16 Apr 2024 10:30:40 -0700 Subject: [PATCH 18/18] use Sprintf to format string --- cmd/start.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/start.go b/cmd/start.go index 8693140..e817d42 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -220,7 +220,7 @@ func ingest(ctx context.Context, bheUrl url.URL, bheClient *http.Client, in <-ch if response, err := bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connection - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) + log.Error(err, fmt.Sprintf("remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries)) rest.ExponentialBackoff(retry) if retry == maxRetries-1 { @@ -288,7 +288,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res, err = bheClient.Do(req); err != nil { if rest.IsClosedConnectionErr(err) { // try again on force closed connections - log.Error(err, "remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries) + log.Error(err, fmt.Sprintf("remote host force closed connection while requesting %s; attempt %d/%d; trying again", req.URL, retry+1, maxRetries)) rest.ExponentialBackoff(retry) continue } @@ -298,7 +298,7 @@ func do(bheClient *http.Client, req *http.Request) (*http.Response, error) { if res.StatusCode >= http.StatusInternalServerError { // Internal server error, backoff and try again. serverError := fmt.Errorf("received server error %d while requesting %v", res.StatusCode, req.URL) - log.Error(serverError, "attempt %d/%d; trying again", retry+1, maxRetries) + log.Error(serverError, fmt.Sprintf("attempt %d/%d; trying again", retry+1, maxRetries)) rest.ExponentialBackoff(retry) continue