Stop source-name worker looping on permanent failures
The access-review source-name worker never marked a source synced when name resolution errored, so it re-claimed the source on every poll and retried at vendor-latency cadence. Two permanently-failing sources generated millions of error logs (Brex /v2/company 403 and Cloudflare /accounts 400) and hammered vendor APIs (8.6M 403s to Brex in 30 days) -- a ban risk, all for best-effort display metadata. Generalize the Google-403 special case: name resolvers now classify a non-2xx response through nameStatusError, which wraps ErrTerminalNameResolution for permanent client errors (400, 401, 403, 404) and returns a plain, retryable error for everything else (5xx, network). The worker treats a terminal error as done -- it keeps the generic name and marks the source synced -- while transient failures keep retrying as before. Also fix the Cloudflare name resolver's own bug: it requested per_page=1, but Cloudflare's List Accounts endpoint requires per_page in 5..50 and 400s otherwise (the driver already uses 50). That 400 was the sole cause of the Cloudflare retry storm; bump it to 50. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
This commit is contained in:
@@ -22,6 +22,7 @@ package drivers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
@@ -31,6 +32,35 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestNameStatusError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
terminal := []int{
|
||||
http.StatusBadRequest,
|
||||
http.StatusUnauthorized,
|
||||
http.StatusForbidden,
|
||||
http.StatusNotFound,
|
||||
}
|
||||
for _, code := range terminal {
|
||||
err := nameStatusError("thing", code)
|
||||
require.Error(t, err)
|
||||
assert.ErrorIs(t, err, ErrTerminalNameResolution, "status %d must be terminal", code)
|
||||
}
|
||||
|
||||
retryable := []int{
|
||||
http.StatusTooManyRequests,
|
||||
http.StatusInternalServerError,
|
||||
http.StatusBadGateway,
|
||||
http.StatusServiceUnavailable,
|
||||
http.StatusGatewayTimeout,
|
||||
}
|
||||
for _, code := range retryable {
|
||||
err := nameStatusError("thing", code)
|
||||
require.Error(t, err)
|
||||
assert.False(t, errors.Is(err, ErrTerminalNameResolution), "status %d must be retryable", code)
|
||||
}
|
||||
}
|
||||
|
||||
// hostRewriter redirects requests to the configured target host so that
|
||||
// resolvers with hardcoded production URLs (api.notion.com, etc.) can be
|
||||
// pointed at an httptest server.
|
||||
|
||||
Reference in New Issue
Block a user