From cf584e536a8cd1a22d9a2131543cff6143b3d891 Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Thu, 25 Jan 2024 12:59:14 -0500 Subject: [PATCH 1/6] Retry HTTP2 INTERNAL_ERRORs --- go.mod | 2 + go.sum | 4 ++ pkg/geoipupdate/geoip_updater.go | 70 +++++++++++++++++++++++++++----- 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index b587bb85..9b521727 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/gofrs/flock v0.8.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.4 + golang.org/x/net v0.20.0 golang.org/x/sync v0.6.0 ) @@ -16,6 +17,7 @@ require ( github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/sys v0.16.0 // indirect + golang.org/x/text v0.14.0 // indirect gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 5648f23b..5dc82478 100644 --- a/go.sum +++ b/go.sum @@ -17,10 +17,14 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo= +golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY= golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/pkg/geoipupdate/geoip_updater.go b/pkg/geoipupdate/geoip_updater.go index 24847709..76b7f8da 100644 --- a/pkg/geoipupdate/geoip_updater.go +++ b/pkg/geoipupdate/geoip_updater.go @@ -5,12 +5,16 @@ package geoipupdate import ( "context" "encoding/json" + "errors" "fmt" "log" "os" "sync" "time" + "github.com/cenkalti/backoff/v4" + "golang.org/x/net/http2" + "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/database" "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/internal" ) @@ -85,20 +89,11 @@ func (c *Client) Run(ctx context.Context) error { for _, editionID := range c.config.EditionIDs { editionID := editionID processFunc := func(ctx context.Context) error { - editionHash, err := writer.GetHash(editionID) + edition, err := c.downloadEdition(ctx, editionID, reader, writer) if err != nil { return err } - edition, err := reader.Read(ctx, editionID, editionHash) - if err != nil { - return err - } - - if err := writer.Write(edition); err != nil { - return err - } - edition.CheckedAt = time.Now().In(time.UTC) mu.Lock() @@ -126,3 +121,58 @@ func (c *Client) Run(ctx context.Context) error { return nil } + +// downloadEdition downloads the file with retries on HTTP2 INTERNAL_ERRORs. +func (c *Client) downloadEdition( + ctx context.Context, + editionID string, + r database.Reader, + w database.Writer, +) (*database.ReadResult, error) { + editionHash, err := w.GetHash(editionID) + if err != nil { + return nil, err + } + + // RetryFor value of 0 means that no retries should be performed. + // Max zero retries has to be set to achieve that + // because the backoff never stops if MaxElapsedTime is zero. + exp := backoff.NewExponentialBackOff() + exp.MaxElapsedTime = c.config.RetryFor + b := backoff.BackOff(exp) + if exp.MaxElapsedTime == 0 { + b = backoff.WithMaxRetries(exp, 0) + } + + var edition *database.ReadResult + err = backoff.RetryNotify( + func() error { + edition, err = r.Read(ctx, editionID, editionHash) + if err != nil { + return backoff.Permanent(err) + } + + if err = w.Write(edition); err != nil { + streamErr := http2.StreamError{} + if errors.As(err, &streamErr) && streamErr.Code.String() == "INTERNAL_ERROR" { + return err + } + + return backoff.Permanent(err) + } + + return nil + }, + b, + func(err error, d time.Duration) { + if c.config.Verbose { + log.Printf("Couldn't download %s, retrying in %v: %v", editionID, d, err) + } + }, + ) + if err != nil { + return nil, err + } + + return edition, nil +} From 80f9b55f7274359fef2fafa48e030955833ed90e Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Thu, 25 Jan 2024 13:52:09 -0500 Subject: [PATCH 2/6] Update LocalFileWriter.Write to return Close errors --- pkg/geoipupdate/database/local_file_writer.go | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/geoipupdate/database/local_file_writer.go b/pkg/geoipupdate/database/local_file_writer.go index 05f14cbd..d76043e0 100644 --- a/pkg/geoipupdate/database/local_file_writer.go +++ b/pkg/geoipupdate/database/local_file_writer.go @@ -46,7 +46,7 @@ func NewLocalFileWriter( } // Write writes the result struct returned by a Reader to a database file. -func (w *LocalFileWriter) Write(result *ReadResult) error { +func (w *LocalFileWriter) Write(result *ReadResult) (err error) { // exit early if we've got the latest database version. if strings.EqualFold(result.OldHash, result.NewHash) { if w.verbose { @@ -56,8 +56,11 @@ func (w *LocalFileWriter) Write(result *ReadResult) error { } defer func() { - if err := result.reader.Close(); err != nil { - log.Printf("closing reader for %s: %+v", result.EditionID, err) + if closeErr := result.reader.Close(); closeErr != nil { + err = errors.Join( + err, + fmt.Errorf("closing reader for %s: %w", result.EditionID, closeErr), + ) } }() @@ -69,34 +72,37 @@ func (w *LocalFileWriter) Write(result *ReadResult) error { return fmt.Errorf("setting up database writer for %s: %w", result.EditionID, err) } defer func() { - if err := fw.close(); err != nil { - log.Printf("closing file writer: %+v", err) + if closeErr := fw.close(); closeErr != nil { + err = errors.Join( + err, + fmt.Errorf("closing file writer: %w", closeErr), + ) } }() - if err := fw.write(result.reader); err != nil { + if err = fw.write(result.reader); err != nil { return fmt.Errorf("writing to the temp file for %s: %w", result.EditionID, err) } // make sure the hash of the temp file matches the expected hash. - if err := fw.validateHash(result.NewHash); err != nil { + if err = fw.validateHash(result.NewHash); err != nil { return fmt.Errorf("validating hash for %s: %w", result.EditionID, err) } // move the temoporary database file into it's final location and // sync the directory. - if err := fw.syncAndRename(databaseFilePath); err != nil { + if err = fw.syncAndRename(databaseFilePath); err != nil { return fmt.Errorf("renaming temp file: %w", err) } // sync database directory. - if err := syncDir(filepath.Dir(databaseFilePath)); err != nil { + if err = syncDir(filepath.Dir(databaseFilePath)); err != nil { return fmt.Errorf("syncing database directory: %w", err) } // check if we need to set the file's modified at time if w.preserveFileTime { - if err := setModifiedAtTime(databaseFilePath, result.ModifiedAt); err != nil { + if err = setModifiedAtTime(databaseFilePath, result.ModifiedAt); err != nil { return err } } From 55ef27abbb64ecdefc5f4714927690d968b1d2d3 Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Thu, 25 Jan 2024 14:18:51 -0500 Subject: [PATCH 3/6] Add HTTP2 INTERNAL_ERROR test --- pkg/geoipupdate/geoip_updater.go | 2 +- pkg/geoipupdate/geoip_updater_test.go | 33 +++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/pkg/geoipupdate/geoip_updater.go b/pkg/geoipupdate/geoip_updater.go index 76b7f8da..8aa9e7d1 100644 --- a/pkg/geoipupdate/geoip_updater.go +++ b/pkg/geoipupdate/geoip_updater.go @@ -154,7 +154,7 @@ func (c *Client) downloadEdition( if err = w.Write(edition); err != nil { streamErr := http2.StreamError{} - if errors.As(err, &streamErr) && streamErr.Code.String() == "INTERNAL_ERROR" { + if errors.As(err, &streamErr) && streamErr.Code == http2.ErrCodeInternal { return err } diff --git a/pkg/geoipupdate/geoip_updater_test.go b/pkg/geoipupdate/geoip_updater_test.go index cf648ab9..876bb897 100644 --- a/pkg/geoipupdate/geoip_updater_test.go +++ b/pkg/geoipupdate/geoip_updater_test.go @@ -10,8 +10,10 @@ import ( "testing" "time" - "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/database" "github.com/stretchr/testify/require" + "golang.org/x/net/http2" + + "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/database" ) // TestClientOutput makes sure that the client outputs the result of it's @@ -77,6 +79,21 @@ func TestClientOutput(t *testing.T) { t.Errorf("database %s was not updated", outputDatabases[i].EditionID) } } + + streamErr := http2.StreamError{ + Code: http2.ErrCodeInternal, + } + c.getWriter = func() (database.Writer, error) { + w := mockWriter{ + WriteFunc: func(_ *database.ReadResult) error { + return streamErr + }, + } + + return &w, nil + } + err = c.Run(context.Background()) + require.ErrorIs(t, err, streamErr) } type mockReader struct { @@ -93,10 +110,18 @@ func (mr *mockReader) Read(_ context.Context, _, _ string) (*database.ReadResult return &res, nil } -type mockWriter struct{} +type mockWriter struct { + WriteFunc func(*database.ReadResult) error +} + +func (w *mockWriter) Write(r *database.ReadResult) error { + if w.WriteFunc != nil { + return w.WriteFunc(r) + } -func (w *mockWriter) Write(_ *database.ReadResult) error { return nil } -func (w mockWriter) GetHash(_ string) (string, error) { return "", nil } + return nil +} +func (w mockWriter) GetHash(_ string) (string, error) { return "", nil } func afterOrEqual(t1, t2 time.Time) bool { return t1.After(t2) || t1.Equal(t2) From f293697f187360815077d98901482ba6bb05d507 Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Fri, 26 Jan 2024 11:14:12 -0500 Subject: [PATCH 4/6] Keep a single retry loop --- pkg/geoipupdate/database/http_reader.go | 36 +------------------------ pkg/geoipupdate/geoip_updater.go | 12 ++++----- pkg/geoipupdate/internal/errors.go | 19 +++++++++++++ 3 files changed, 26 insertions(+), 41 deletions(-) diff --git a/pkg/geoipupdate/database/http_reader.go b/pkg/geoipupdate/database/http_reader.go index e2f93428..78677016 100644 --- a/pkg/geoipupdate/database/http_reader.go +++ b/pkg/geoipupdate/database/http_reader.go @@ -14,8 +14,6 @@ import ( "strconv" "time" - "github.com/cenkalti/backoff/v4" - "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/internal" "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/vars" ) @@ -71,39 +69,7 @@ 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) { - var result *ReadResult - var err error - - // RetryFor value of 0 means that no retries should be performed. - // Max zero retries has to be set to achieve that - // because the backoff never stops if MaxElapsedTime is zero. - exp := backoff.NewExponentialBackOff() - exp.MaxElapsedTime = r.retryFor - b := backoff.BackOff(exp) - if exp.MaxElapsedTime == 0 { - b = backoff.WithMaxRetries(exp, 0) - } - 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) - } - }, - ) + result, err := r.get(ctx, editionID, hash) 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 8aa9e7d1..e80f03ec 100644 --- a/pkg/geoipupdate/geoip_updater.go +++ b/pkg/geoipupdate/geoip_updater.go @@ -5,7 +5,6 @@ package geoipupdate import ( "context" "encoding/json" - "errors" "fmt" "log" "os" @@ -13,7 +12,6 @@ import ( "time" "github.com/cenkalti/backoff/v4" - "golang.org/x/net/http2" "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/database" "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/internal" @@ -147,14 +145,16 @@ func (c *Client) downloadEdition( var edition *database.ReadResult err = backoff.RetryNotify( func() error { - edition, err = r.Read(ctx, editionID, editionHash) - if err != nil { + if edition, err = r.Read(ctx, editionID, editionHash); err != nil { + if internal.IsTemporaryError(err) { + return err + } + return backoff.Permanent(err) } if err = w.Write(edition); err != nil { - streamErr := http2.StreamError{} - if errors.As(err, &streamErr) && streamErr.Code == http2.ErrCodeInternal { + if internal.IsTemporaryError(err) { return err } diff --git a/pkg/geoipupdate/internal/errors.go b/pkg/geoipupdate/internal/errors.go index 9194c85b..8d280412 100644 --- a/pkg/geoipupdate/internal/errors.go +++ b/pkg/geoipupdate/internal/errors.go @@ -2,7 +2,10 @@ package internal import ( + "errors" "fmt" + + "golang.org/x/net/http2" ) // HTTPError is an error from performing an HTTP request. @@ -14,3 +17,19 @@ type HTTPError struct { 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 { + 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 { + return true + } + + return false +} From a850ca9c7981e56224382782758c580d9bd99c6f Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Fri, 26 Jan 2024 11:37:29 -0500 Subject: [PATCH 5/6] Revert HTTPReader retry changes --- pkg/geoipupdate/database/http_reader.go | 38 +++++++++++++++++++- pkg/geoipupdate/geoip_updater.go | 17 ++++----- pkg/geoipupdate/internal/errors.go | 14 ++------ pkg/geoipupdate/internal/errors_test.go | 47 +++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 20 deletions(-) create mode 100644 pkg/geoipupdate/internal/errors_test.go diff --git a/pkg/geoipupdate/database/http_reader.go b/pkg/geoipupdate/database/http_reader.go index 78677016..fda9d368 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,41 @@ 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 + } + + if internal.IsPermanentError(err) { + 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..8d7c56be 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 } @@ -120,7 +121,7 @@ func (c *Client) Run(ctx context.Context) error { return nil } -// downloadEdition downloads the file with retries on HTTP2 INTERNAL_ERRORs. +// downloadEdition downloads the file with retries. func (c *Client) downloadEdition( ctx context.Context, editionID string, @@ -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 } diff --git a/pkg/geoipupdate/internal/errors_test.go b/pkg/geoipupdate/internal/errors_test.go new file mode 100644 index 00000000..f5ea2109 --- /dev/null +++ b/pkg/geoipupdate/internal/errors_test.go @@ -0,0 +1,47 @@ +package internal + +import ( + "net/http" + "testing" + + "golang.org/x/net/http2" +) + +func TestIsPermanentError(t *testing.T) { + tt := map[string]struct { + err error + want bool + }{ + "HTTP2 INTERNAL_ERROR": { + err: http2.StreamError{ + Code: http2.ErrCodeInternal, + }, + want: false, + }, + "bad gateway": { + err: HTTPError{ + StatusCode: http.StatusBadGateway, + }, + want: false, + }, + "bad request": { + err: HTTPError{ + StatusCode: http.StatusBadRequest, + }, + want: true, + }, + "nil": { + err: nil, + want: false, + }, + } + + for name, tc := range tt { + t.Run(name, func(t *testing.T) { + got := IsPermanentError(tc.err) + if tc.want != got { + t.Errorf("expected %v got %v", tc.want, got) + } + }) + } +} From fe45ad762035278663ea44d6c17ca1b405ad9a4f Mon Sep 17 00:00:00 2001 From: Marsel Mavletkulov Date: Tue, 30 Jan 2024 15:23:19 -0500 Subject: [PATCH 6/6] Remove retries from HTTPReader (breaking change) --- CHANGELOG.md | 5 +++ pkg/geoipupdate/database/http_reader.go | 42 +------------------- pkg/geoipupdate/database/http_reader_test.go | 1 - pkg/geoipupdate/geoip_updater.go | 2 - 4 files changed, 6 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c40e2fc..74c6ea4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # CHANGELOG +## 7.0.0 + +* `HTTPReader` no longer retries on HTTP errors and therefore + `retryFor` was removed from `NewHTTPReader`. + ## 6.1.0 (2024-01-09) * `geoipupdate` now sets the version in the `User-Agent` header to the diff --git a/pkg/geoipupdate/database/http_reader.go b/pkg/geoipupdate/database/http_reader.go index fda9d368..7cb234fe 100644 --- a/pkg/geoipupdate/database/http_reader.go +++ b/pkg/geoipupdate/database/http_reader.go @@ -14,8 +14,6 @@ import ( "strconv" "time" - "github.com/cenkalti/backoff/v4" - "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/internal" "github.com/maxmind/geoipupdate/v6/pkg/geoipupdate/vars" ) @@ -33,8 +31,6 @@ type HTTPReader struct { accountID int // licenseKey is used for request auth. licenseKey string - // retryFor sets the timeout for when a request can no longuer be retried. - retryFor time.Duration // verbose turns on/off debug logs. verbose bool } @@ -46,7 +42,6 @@ func NewHTTPReader( path string, accountID int, licenseKey string, - retryFor time.Duration, verbose bool, ) Reader { transport := http.DefaultTransport @@ -60,7 +55,6 @@ func NewHTTPReader( path: path, accountID: accountID, licenseKey: licenseKey, - retryFor: retryFor, verbose: verbose, } } @@ -71,41 +65,7 @@ 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) { - 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 - } - - if internal.IsPermanentError(err) { - 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) - } - }, - ) + result, err := r.get(ctx, editionID, hash) if err != nil { return nil, fmt.Errorf("getting update for %s: %w", editionID, err) } diff --git a/pkg/geoipupdate/database/http_reader_test.go b/pkg/geoipupdate/database/http_reader_test.go index d09fdf8b..7c1b0a46 100644 --- a/pkg/geoipupdate/database/http_reader_test.go +++ b/pkg/geoipupdate/database/http_reader_test.go @@ -121,7 +121,6 @@ func TestHTTPReader(t *testing.T) { server.URL, // fixed, as the server is mocked above. 10, // fixed, as it's not valuable for the purpose of the test. "license", // fixed, as it's not valuable for the purpose of the test. - 0, // zero means no retries. false, // verbose ) diff --git a/pkg/geoipupdate/geoip_updater.go b/pkg/geoipupdate/geoip_updater.go index 8d7c56be..88f11754 100644 --- a/pkg/geoipupdate/geoip_updater.go +++ b/pkg/geoipupdate/geoip_updater.go @@ -34,8 +34,6 @@ func NewClient(config *Config) *Client { config.URL, config.AccountID, config.LicenseKey, - // Disable retries in Reader because Client handles retries itself. - 0, config.Verbose, ), nil }