From 22d3367e24c68889a8bc09eaac15584bf3755ab2 Mon Sep 17 00:00:00 2001 From: Javier Maestro Date: Tue, 5 Nov 2024 16:32:13 +0000 Subject: [PATCH] feat: improve httputil error messages (#624) * add attempt count to the timeout error message so it's clear the timeout is due to the total time accumulated during retries. * add lastFailure to the timeout error message so we can get a clear reason for the retries (or, at the very least, the last retry). Otherwise, the error message obscures the real reason(s) for the retries and potentially misleads users into thinking that the errors could be e.g. network related (more typical of a timeout) rather than other types of errors (e.g. "tls: failed to verify certificate", etc.) --- httputil/httputil.go | 2 +- httputil/httputil_test.go | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/httputil/httputil.go b/httputil/httputil.go index a2256551..c020e064 100644 --- a/httputil/httputil.go +++ b/httputil/httputil.go @@ -113,7 +113,7 @@ func get(url, auth string) (*http.Response, error) { nextTryAt := RetryClock.Now().Add(waitFor) if nextTryAt.After(deadline) { - return nil, fmt.Errorf("unable to complete request to %s within %v", url, MaxRequestDuration) + return nil, fmt.Errorf("unable to complete %d requests to %s within %v. Most recent failure: %s", attempt+1, url, MaxRequestDuration, lastFailure) } if attempt < MaxRetries { RetryClock.Sleep(waitFor) diff --git a/httputil/httputil_test.go b/httputil/httputil_test.go index 254e4298..1a3f2281 100644 --- a/httputil/httputil_test.go +++ b/httputil/httputil_test.go @@ -4,6 +4,7 @@ import ( "errors" "net/http" "strconv" + "strings" "testing" "time" ) @@ -219,10 +220,13 @@ func TestDeadlineExceeded(t *testing.T) { t.Fatal("Expected request to fail with code 500") } - wanted := "could not fetch http://bar: unable to complete request to http://bar within 8s" + wantedPrefix := "could not fetch http://bar: unable to complete " + wantedSuffix := " requests to http://bar within 8s. Most recent failure: HTTP 500" + got := err.Error() - if wanted != got { - t.Fatalf("Expected error %q, but got %q", wanted, got) + sameError := strings.HasPrefix(got, wantedPrefix) && strings.HasSuffix(got, wantedSuffix) + if !sameError { + t.Fatalf("Expected error %q, but got %q", wantedPrefix+"???"+wantedSuffix, got) } }