From 3abc22fa15c07c9565377ed6cd5ebdaf1f98fd7c Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Fri, 26 Jan 2024 11:37:29 -0500 Subject: [PATCH] Revert HTTPReader retry changes --- pkg/geoipupdate/database/http_reader.go | 39 ++++++++++++++++++++++++- pkg/geoipupdate/geoip_updater.go | 15 +++++----- pkg/geoipupdate/internal/errors.go | 14 ++------- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/pkg/geoipupdate/database/http_reader.go b/pkg/geoipupdate/database/http_reader.go index 78677016..347fd6f7 100644 --- a/pkg/geoipupdate/database/http_reader.go +++ b/pkg/geoipupdate/database/http_reader.go @@ -14,6 +14,8 @@ import ( "strconv" "time" + "github.com/cenkalti/backoff/v4" + "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/internal" "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/vars" ) @@ -69,7 +71,42 @@ func NewHTTPReader( // It's the responsibility of the Writer to close the io.ReadCloser // included in the response after consumption. func (r *HTTPReader) Read(ctx context.Context, editionID, hash string) (*ReadResult, error) { - result, err := r.get(ctx, editionID, hash) + var result *ReadResult + var err error + + if r.retryFor == 0 { + if result, err = r.get(ctx, editionID, hash); err != nil { + return nil, fmt.Errorf("getting update for %s: %w", editionID, err) + } + + return result, nil + } + + exp := backoff.NewExponentialBackOff() + exp.MaxElapsedTime = r.retryFor + b := backoff.BackOff(exp) + + err = backoff.RetryNotify( + func() error { + result, err = r.get(ctx, editionID, hash) + if err == nil { + return nil + } + + var httpErr internal.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 { + return backoff.Permanent(err) + } + + return err + }, + b, + func(err error, d time.Duration) { + if r.verbose { + log.Printf("Couldn't download %s, retrying in %v: %v", editionID, d, err) + } + }, + ) if err != nil { return nil, fmt.Errorf("getting update for %s: %w", editionID, err) } diff --git a/pkg/geoipupdate/geoip_updater.go b/pkg/geoipupdate/geoip_updater.go index e80f03ec..1c5b1436 100644 --- a/pkg/geoipupdate/geoip_updater.go +++ b/pkg/geoipupdate/geoip_updater.go @@ -34,7 +34,8 @@ func NewClient(config *Config) *Client { config.URL, config.AccountID, config.LicenseKey, - config.RetryFor, + // Disable retries in Reader because Client handles retries itself. + 0, config.Verbose, ), nil } @@ -146,19 +147,19 @@ func (c *Client) downloadEdition( err = backoff.RetryNotify( func() error { if edition, err = r.Read(ctx, editionID, editionHash); err != nil { - if internal.IsTemporaryError(err) { - return err + if internal.IsPermanentError(err) { + return backoff.Permanent(err) } - return backoff.Permanent(err) + return err } if err = w.Write(edition); err != nil { - if internal.IsTemporaryError(err) { - return err + if internal.IsPermanentError(err) { + return backoff.Permanent(err) } - return backoff.Permanent(err) + return err } return nil diff --git a/pkg/geoipupdate/internal/errors.go b/pkg/geoipupdate/internal/errors.go index 8d280412..8d487c64 100644 --- a/pkg/geoipupdate/internal/errors.go +++ b/pkg/geoipupdate/internal/errors.go @@ -4,8 +4,6 @@ package internal import ( "errors" "fmt" - - "golang.org/x/net/http2" ) // HTTPError is an error from performing an HTTP request. @@ -18,16 +16,10 @@ func (h HTTPError) Error() string { return fmt.Sprintf("received HTTP status code: %d: %s", h.StatusCode, h.Body) } -// IsTemporaryError returns true if the error is temporary. -func IsTemporaryError(err error) bool { +// IsPermanentError returns true if the error is non-retriable. +func IsPermanentError(err error) bool { var httpErr HTTPError - if errors.As(err, &httpErr) { - isPermanent := httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 - return !isPermanent - } - - var streamErr http2.StreamError - if errors.As(err, &streamErr) && streamErr.Code == http2.ErrCodeInternal { + if errors.As(err, &httpErr) && httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 { return true }