Surface stale Sentry slug in ListAccounts error
ListAccounts on a connector whose stored slug is no longer accessible to its OAuth token currently returns "cannot fetch sentry members: unexpected status 404" -- opaque, and indistinguishable from a real Sentry outage. The campaign source-fetch worker records that string verbatim as the customer-visible LastError, with no hint that the connector itself needs reconnection. queryMembers now returns a sentinel errSentryOrgNotAccessible on 404, and ListAccounts wraps it with the slug and a directive to reconnect. errors.Is preserves the chain so future callers can branch on the permanent-config-failure case without string matching. No auto-recovery: the only safe slug is one the customer explicitly chose. Picking a different visible org would silently rebind the source to the wrong tenant. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
This commit is contained in:
@@ -17,6 +17,7 @@ package drivers
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -26,6 +27,10 @@ import (
|
|||||||
"go.probo.inc/probo/pkg/rfc5988"
|
"go.probo.inc/probo/pkg/rfc5988"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// errSentryOrgNotAccessible signals a 404 scoped under an organization
|
||||||
|
// slug; Sentry uses 404 (not 403) so this also covers revoked memberships.
|
||||||
|
var errSentryOrgNotAccessible = errors.New("sentry organization is not accessible by this connector's token")
|
||||||
|
|
||||||
// SentryDriver fetches organization members from Sentry via Bearer
|
// SentryDriver fetches organization members from Sentry via Bearer
|
||||||
// token-authenticated REST API requests.
|
// token-authenticated REST API requests.
|
||||||
type SentryDriver struct {
|
type SentryDriver struct {
|
||||||
@@ -97,6 +102,10 @@ func (d *SentryDriver) ListAccounts(ctx context.Context) ([]AccountRecord, error
|
|||||||
for range maxPaginationPages {
|
for range maxPaginationPages {
|
||||||
members, linkHeader, err := d.queryMembers(ctx, nextURL)
|
members, linkHeader, err := d.queryMembers(ctx, nextURL)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
if errors.Is(err, errSentryOrgNotAccessible) {
|
||||||
|
return nil, fmt.Errorf("sentry organization %q is not accessible; reconnect the connector with the correct organization: %w", orgSlug, err)
|
||||||
|
}
|
||||||
|
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -178,6 +187,10 @@ func (d *SentryDriver) queryMembers(ctx context.Context, url string) ([]sentryMe
|
|||||||
_ = httpResp.Body.Close()
|
_ = httpResp.Body.Close()
|
||||||
}()
|
}()
|
||||||
|
|
||||||
|
if httpResp.StatusCode == http.StatusNotFound {
|
||||||
|
return nil, "", errSentryOrgNotAccessible
|
||||||
|
}
|
||||||
|
|
||||||
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 {
|
||||||
return nil, "", fmt.Errorf("cannot fetch sentry members: unexpected status %d", httpResp.StatusCode)
|
return nil, "", fmt.Errorf("cannot fetch sentry members: unexpected status %d", httpResp.StatusCode)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -48,6 +48,27 @@ func TestSentryDriver(t *testing.T) {
|
|||||||
assert.NotEmpty(t, r.Role)
|
assert.NotEmpty(t, r.Role)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSentryDriverListAccountsStaleSlug(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-old/members", r.URL.Path)
|
||||||
|
w.WriteHeader(http.StatusNotFound)
|
||||||
|
_, _ = w.Write([]byte(`{"detail":"The requested resource does not exist"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
client := &http.Client{Transport: &hostRewriter{target: srv.URL}}
|
||||||
|
|
||||||
|
_, err := NewSentryDriver(client, "acme-old").ListAccounts(context.Background())
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Contains(t, err.Error(), `"acme-old"`)
|
||||||
|
assert.Contains(t, err.Error(), "not accessible")
|
||||||
|
assert.Contains(t, err.Error(), "reconnect")
|
||||||
|
assert.ErrorIs(t, err, errSentryOrgNotAccessible)
|
||||||
|
}
|
||||||
|
|
||||||
func TestSentryDriverListAccountsAutoDiscoversSlug(t *testing.T) {
|
func TestSentryDriverListAccountsAutoDiscoversSlug(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user