Revert HTTPReader retry changes

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)
 	}

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

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
 	}
 

pkg/geoipupdate/internal/errors_test.go

@@ -0,0 +1,43 @@
+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,
+		},
+	}
+
+	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)
+			}
+		})
+	}
+}