From 7ff3b0b5278f98668492be79acc6e5a86004541e Mon Sep 17 00:00:00 2001 From: MudassarTariq Date: Thu, 12 Nov 2026 22:43:28 +0500 Subject: [PATCH] fix: stop retrying 429 responses --- README.md | 2 +- docs/configuration.md | 2 +- docs/troubleshooting.md | 2 +- internal/ipgeolocation/client_test.go | 2 +- internal/ipgeolocation/errors.go | 3 ++- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 34fa34c..81de2a4 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ spec: | `endpoint` | string | `https://api.ipgeolocation.io` | API base URL | | `timeout` | string | `30s` | How long to wait for one API request, such as `30s` or `1m`. Must be greater than zero. | | `max_response_mb` | integer | `256` | Largest response to read, in MB. Increase this for large ASN results. Must be a positive whole number. | -| `retry_attempts` | int | `3` | Retries for transient errors such as 429 and 5xx | +| `retry_attempts` | int | `3` | Retries for transient errors such as 5xx responses and network failures | | `concurrency` | int | `10` | Tables synced in parallel | | `rate_limit` | int | `15` | Maximum API requests per second | | `user_agent` | string | `cq-source-ipgeolocation/1.0` | User-Agent header sent with every request | diff --git a/docs/configuration.md b/docs/configuration.md index 946ac38..7b384d8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -159,7 +159,7 @@ spec: The plugin holds a token bucket limiter set to `rate_limit` requests per second with a burst of the same size. The API has no per-second rate limits, so `rate_limit` only paces the sync. A 429 means your credits are used up. -Retries cover 429, 500, 502, 503 and 504 responses along with network level failures. Backoff doubles each time, so the waits are 1 second, then 2, then 4. Other 4xx responses are returned immediately, because retrying a 400 or a 401 will never succeed. +Retries cover 500, 502, 503 and 504 responses along with network level failures. Backoff doubles each time, so the waits are 1 second, then 2, then 4. 4xx responses, including 429, are returned immediately, because retrying a 400, a 401 or a 429 will never succeed. `timeout` applies per HTTP request, not to the sync as a whole. Raise it if you enable `hostname_lookup: "live"`, since live reverse DNS is the slowest thing the API does. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 7fd0b46..81f624c 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -29,7 +29,7 @@ If `${IPGEOLOCATION_API_KEY}` reaches the plugin unexpanded, the shell running ` | 429 | No credits left: the free plan's daily credits, or a paid plan's credits with overage off or at its cap (the API has no per-second rate limits) | Check remaining credits in the dashboard. Overage is on by default on all paid plans and is switched with Allow Surcharge Usage | | 5xx | Server side fault | Retried automatically. Persistent failures are worth reporting | -The plugin retries 429, 500, 502, 503 and 504 along with network failures, backing off 1 second, then 2, then 4. Other 4xx responses are returned at once, because a 400 or a 401 will not improve on a second attempt. +The plugin retries 500, 502, 503 and 504 along with network failures, backing off 1 second, then 2, then 4. 4xx responses, including 429, are returned at once, because a 400, a 401 or a 429 will not improve on a second attempt. ## A Table Is Empty After a Successful Sync diff --git a/internal/ipgeolocation/client_test.go b/internal/ipgeolocation/client_test.go index 3a6e3f2..b13ca60 100644 --- a/internal/ipgeolocation/client_test.go +++ b/internal/ipgeolocation/client_test.go @@ -189,7 +189,7 @@ func TestAPIErrorRetryable(t *testing.T) { code int retryable bool }{ - {429, true}, + {429, false}, {500, true}, {502, true}, {503, true}, diff --git a/internal/ipgeolocation/errors.go b/internal/ipgeolocation/errors.go index bdc94f7..275d341 100644 --- a/internal/ipgeolocation/errors.go +++ b/internal/ipgeolocation/errors.go @@ -20,9 +20,10 @@ func (e *APIError) Error() string { } // IsRetryable returns true for transient HTTP errors that are worth retrying. +// A 429 is not one: it means no credits are left, and a retry cannot add credits. func (e *APIError) IsRetryable() bool { switch e.StatusCode { - case 429, 500, 502, 503, 504: + case 500, 502, 503, 504: return true default: return false