From 3a0bcade9952623712006d1f264a0497322242b5 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Mon, 9 Feb 2026 22:44:14 -0600 Subject: [PATCH 1/2] feat: enhance HTTPError handling with retry logic - Updated the Temporary method to include additional status codes (408, 429) for temporary errors. - Added RetryAfter method to calculate wait time based on rate limit headers in the response. --- errors/errors.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/errors/errors.go b/errors/errors.go index ba99374..bd9184d 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -5,6 +5,8 @@ import ( "fmt" "io" "net/http" + "strconv" + "time" ) // HTTPError preserves HTTP response details @@ -24,7 +26,19 @@ func (e *HTTPError) Error() string { // Temporary implements net.Error interface func (e *HTTPError) Temporary() bool { - return e.Response.StatusCode >= 500 + code := e.Response.StatusCode + return code == 408 || code == 429 || code >= 500 +} + +// RetryAfter returns how long to wait before retrying based on +// rate limit headers in the response +func (e *HTTPError) RetryAfter() time.Duration { + if v := e.Response.Header.Get("x-internxt-ratelimit-reset"); v != "" { + if ms, err := strconv.Atoi(v); err == nil && ms > 0 { + return time.Duration(ms) * time.Millisecond + } + } + return 0 } // StatusCode returns the HTTP status code From d1845f43552fa74485eeadbbec3142002ac47d47 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Thu, 12 Feb 2026 20:54:13 -0600 Subject: [PATCH 2/2] fix: refactor rate limit wait time parsing, add tests - Introduced comprehensive tests for the RetryAfter method in the HTTPError struct, covering various scenarios including valid seconds, HTTP date formats, absence of headers, and invalid values. - Added tests to ensure correct behavior for temporary errors (HTTP status codes 408 and 429) in the GetUsage and GetLimit functions, verifying that they return the expected HTTPError instances. --- errors/errors.go | 11 ++-- errors/errors_test.go | 114 ++++++++++++++++++++++++++++++++++++++++++ users/limit.go | 5 +- users/usage.go | 5 +- users/users_test.go | 93 ++++++++++++++++++++++++++++++++++ 5 files changed, 219 insertions(+), 9 deletions(-) create mode 100644 errors/errors_test.go diff --git a/errors/errors.go b/errors/errors.go index bd9184d..76d1ce1 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -33,9 +33,14 @@ func (e *HTTPError) Temporary() bool { // RetryAfter returns how long to wait before retrying based on // rate limit headers in the response func (e *HTTPError) RetryAfter() time.Duration { - if v := e.Response.Header.Get("x-internxt-ratelimit-reset"); v != "" { - if ms, err := strconv.Atoi(v); err == nil && ms > 0 { - return time.Duration(ms) * time.Millisecond + if v := e.Response.Header.Get("Retry-After"); v != "" { + if seconds, err := strconv.Atoi(v); err == nil && seconds > 0 { + return time.Duration(seconds) * time.Second + } + if t, err := http.ParseTime(v); err == nil { + if delay := time.Until(t); delay > 0 { + return delay + } } } return 0 diff --git a/errors/errors_test.go b/errors/errors_test.go new file mode 100644 index 0000000..0f7fad5 --- /dev/null +++ b/errors/errors_test.go @@ -0,0 +1,114 @@ +package errors + +import ( + "net/http" + "testing" + "time" +) + +func newHTTPResponse(statusCode int, headers map[string]string) *http.Response { + h := make(http.Header) + for k, v := range headers { + h.Set(k, v) + } + return &http.Response{ + StatusCode: statusCode, + Header: h, + } +} + +func TestRetryAfter_Seconds(t *testing.T) { + e := &HTTPError{Response: newHTTPResponse(429, map[string]string{ + "Retry-After": "30", + })} + + got := e.RetryAfter() + want := 30 * time.Second + if got != want { + t.Errorf("RetryAfter() = %v, want %v", got, want) + } +} + +func TestRetryAfter_HTTPDate(t *testing.T) { + future := time.Now().Add(45 * time.Second) + dateStr := future.UTC().Format(http.TimeFormat) + + e := &HTTPError{Response: newHTTPResponse(429, map[string]string{ + "Retry-After": dateStr, + })} + + got := e.RetryAfter() + if got < 43*time.Second || got > 47*time.Second { + t.Errorf("RetryAfter() = %v, want ~45s", got) + } +} + +func TestRetryAfter_NoHeader(t *testing.T) { + e := &HTTPError{Response: newHTTPResponse(429, nil)} + + got := e.RetryAfter() + if got != 0 { + t.Errorf("RetryAfter() = %v, want 0", got) + } +} + +func TestRetryAfter_InvalidValue(t *testing.T) { + e := &HTTPError{Response: newHTTPResponse(429, map[string]string{ + "Retry-After": "not-a-number", + })} + + got := e.RetryAfter() + if got != 0 { + t.Errorf("RetryAfter() = %v, want 0 for unparseable value", got) + } +} + +func TestRetryAfter_ZeroSeconds(t *testing.T) { + e := &HTTPError{Response: newHTTPResponse(429, map[string]string{ + "Retry-After": "0", + })} + + got := e.RetryAfter() + if got != 0 { + t.Errorf("RetryAfter() = %v, want 0", got) + } +} + +func TestRetryAfter_PastHTTPDate(t *testing.T) { + past := time.Now().Add(-10 * time.Second) + dateStr := past.UTC().Format(http.TimeFormat) + + e := &HTTPError{Response: newHTTPResponse(429, map[string]string{ + "Retry-After": dateStr, + })} + + got := e.RetryAfter() + if got != 0 { + t.Errorf("RetryAfter() = %v, want 0 for past date", got) + } +} + +func TestTemporary(t *testing.T) { + tests := []struct { + code int + want bool + }{ + {200, false}, + {400, false}, + {401, false}, + {403, false}, + {404, false}, + {408, true}, + {429, true}, + {500, true}, + {502, true}, + {503, true}, + } + + for _, tc := range tests { + e := &HTTPError{Response: newHTTPResponse(tc.code, nil)} + if got := e.Temporary(); got != tc.want { + t.Errorf("Temporary() for status %d = %v, want %v", tc.code, got, tc.want) + } + } +} diff --git a/users/limit.go b/users/limit.go index 3b08246..02a27a3 100644 --- a/users/limit.go +++ b/users/limit.go @@ -4,10 +4,10 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "github.com/internxt/rclone-adapter/config" + sdkerrors "github.com/internxt/rclone-adapter/errors" ) type LimitResponse struct { @@ -30,8 +30,7 @@ func GetLimit(ctx context.Context, cfg *config.Config) (*LimitResponse, error) { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("GET %s returned %d: %s", url, resp.StatusCode, string(body)) + return nil, sdkerrors.NewHTTPError(resp, "get limit") } var limit LimitResponse diff --git a/users/usage.go b/users/usage.go index 09ef4c3..7dcc824 100644 --- a/users/usage.go +++ b/users/usage.go @@ -4,10 +4,10 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "github.com/internxt/rclone-adapter/config" + sdkerrors "github.com/internxt/rclone-adapter/errors" ) type UsageResponse struct { @@ -30,8 +30,7 @@ func GetUsage(ctx context.Context, cfg *config.Config) (*UsageResponse, error) { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("GET %s returned %d: %s", url, resp.StatusCode, string(body)) + return nil, sdkerrors.NewHTTPError(resp, "get usage") } var usage UsageResponse diff --git a/users/users_test.go b/users/users_test.go index a9204e0..f157fed 100644 --- a/users/users_test.go +++ b/users/users_test.go @@ -3,10 +3,14 @@ package users import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" "testing" + "time" + + sdkerrors "github.com/internxt/rclone-adapter/errors" ) func TestGetUsage(t *testing.T) { @@ -260,3 +264,92 @@ func TestGetLimitHTTPClientError(t *testing.T) { t.Errorf("expected error to contain 'failed to execute', got %q", err.Error()) } } + +// TestGetUsage429ReturnsHTTPError verifies that a 429 from GetUsage returns +// an *sdkerrors.HTTPError +func TestGetUsage429ReturnsHTTPError(t *testing.T) { + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "15") + w.WriteHeader(http.StatusTooManyRequests) + w.Write([]byte(`{"error":"rate limited"}`)) + })) + defer mockServer.Close() + + cfg := newTestConfig(mockServer.URL) + _, err := GetUsage(context.Background(), cfg) + if err == nil { + t.Fatal("expected error, got nil") + } + + var httpErr *sdkerrors.HTTPError + if !errors.As(err, &httpErr) { + t.Fatalf("expected *sdkerrors.HTTPError, got %T: %v", err, err) + } + if httpErr.StatusCode() != 429 { + t.Errorf("expected status 429, got %d", httpErr.StatusCode()) + } + if !httpErr.Temporary() { + t.Error("expected Temporary() = true for 429") + } + if got := httpErr.RetryAfter(); got != 15*time.Second { + t.Errorf("RetryAfter() = %v, want 15s", got) + } +} + +// TestGetLimit429ReturnsHTTPError verifies that a 429 from GetLimit returns +// an *sdkerrors.HTTPError +func TestGetLimit429ReturnsHTTPError(t *testing.T) { + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "60") + w.WriteHeader(http.StatusTooManyRequests) + w.Write([]byte(`{"error":"rate limited"}`)) + })) + defer mockServer.Close() + + cfg := newTestConfig(mockServer.URL) + _, err := GetLimit(context.Background(), cfg) + if err == nil { + t.Fatal("expected error, got nil") + } + + var httpErr *sdkerrors.HTTPError + if !errors.As(err, &httpErr) { + t.Fatalf("expected *sdkerrors.HTTPError, got %T: %v", err, err) + } + if httpErr.StatusCode() != 429 { + t.Errorf("expected status 429, got %d", httpErr.StatusCode()) + } + if !httpErr.Temporary() { + t.Error("expected Temporary() = true for 429") + } + if got := httpErr.RetryAfter(); got != 60*time.Second { + t.Errorf("RetryAfter() = %v, want 60s", got) + } +} + +// TestGetUsage408ReturnsHTTPError verifies that a 408 timeout returns +// a retryable *sdkerrors.HTTPError. +func TestGetUsage408ReturnsHTTPError(t *testing.T) { + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusRequestTimeout) + w.Write([]byte(`{"message":"Request timed out"}`)) + })) + defer mockServer.Close() + + cfg := newTestConfig(mockServer.URL) + _, err := GetUsage(context.Background(), cfg) + if err == nil { + t.Fatal("expected error, got nil") + } + + var httpErr *sdkerrors.HTTPError + if !errors.As(err, &httpErr) { + t.Fatalf("expected *sdkerrors.HTTPError, got %T: %v", err, err) + } + if httpErr.StatusCode() != 408 { + t.Errorf("expected status 408, got %d", httpErr.StatusCode()) + } + if !httpErr.Temporary() { + t.Error("expected Temporary() = true for 408") + } +}