Stop source-name worker from looping on stale Sentry slug
The source-name worker re-claims any AccessSource whose name resolver
returns an error. kit/worker drains tasks in a tight inner loop per
tick, so a permanently-failing resolver hammers Sentry as fast as the
HTTP RTT allows -- in prod, ~5 errors/s for 12h+ on one stale slug.
A 404 from /api/0/organizations/{slug} means the stored slug is no
longer visible to the OAuth token (org renamed/deleted, membership
changed). Retrying cannot recover the name, so return ("", nil) like
the openai and intercom resolvers already do: the worker marks the
row synced, the flood stops, and the source keeps its generic name.
Other non-2xx (401/403/5xx) stay retryable so OAuth refresh and
transient outages still get another chance.
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
This commit is contained in:
@@ -471,6 +471,13 @@ func (r *sentryNameResolver) ResolveInstanceName(ctx context.Context) (string, e
|
||||
|
||||
defer func() { _ = httpResp.Body.Close() }()
|
||||
|
||||
// 404 means the stored slug is no longer visible to this token.
|
||||
// Treat as terminal so the worker stops looping; other non-2xx
|
||||
// stay retryable for token refresh / transient outages.
|
||||
if httpResp.StatusCode == http.StatusNotFound {
|
||||
return "", nil
|
||||
}
|
||||
|
||||
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
||||
return "", fmt.Errorf("cannot fetch sentry organization: unexpected status %d", httpResp.StatusCode)
|
||||
}
|
||||
|
||||
@@ -102,3 +102,93 @@ func TestNotionNameResolver(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSentryNameResolver(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("empty slug returns nothing without HTTP call", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := &http.Client{Transport: roundTripperFunc(func(*http.Request) (*http.Response, error) {
|
||||
t.Fatalf("resolver should not make an HTTP call for an empty slug")
|
||||
return nil, nil
|
||||
})}
|
||||
|
||||
got, err := NewSentryNameResolver(client, "").ResolveInstanceName(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, got)
|
||||
})
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
status int
|
||||
body string
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "200 returns name",
|
||||
status: http.StatusOK,
|
||||
body: `{"slug":"acme","name":"Acme Inc"}`,
|
||||
want: "Acme Inc",
|
||||
},
|
||||
{
|
||||
name: "404 is terminal (no error, no name)",
|
||||
status: http.StatusNotFound,
|
||||
body: `{"detail":"The requested resource does not exist"}`,
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "401 is retryable",
|
||||
status: http.StatusUnauthorized,
|
||||
body: `{"detail":"Authentication credentials were not provided."}`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "403 is retryable",
|
||||
status: http.StatusForbidden,
|
||||
body: `{"detail":"You do not have permission to perform this action."}`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "500 is retryable",
|
||||
status: http.StatusInternalServerError,
|
||||
body: `{"detail":"Internal Server Error"}`,
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, http.MethodGet, r.Method)
|
||||
assert.Equal(t, "/api/0/organizations/acme", r.URL.Path)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(tc.status)
|
||||
_, _ = w.Write([]byte(tc.body))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
client := &http.Client{Transport: &hostRewriter{target: srv.URL}}
|
||||
|
||||
got, err := NewSentryNameResolver(client, "acme").ResolveInstanceName(context.Background())
|
||||
if tc.wantErr {
|
||||
require.Error(t, err)
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// roundTripperFunc adapts a function into an http.RoundTripper, useful for
|
||||
// asserting that a resolver short-circuits before making any HTTP call.
|
||||
type roundTripperFunc func(*http.Request) (*http.Response, error)
|
||||
|
||||
func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) {
|
||||
return f(r)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user