From c4c6472f74b4bb538efa93bc3def5ec85ca650e2 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Wed, 26 Aug 2026 16:35:47 -0300 Subject: [PATCH 1/9] Map failed OAuth2 client-credentials token exchange to a grpc status A rejected client-credentials token exchange surfaces as *oauth2.RetrieveError wrapping the token endpoint's real HTTP response, but wrapTransientNetworkError had no case for it, so it fell through unclassified and callers (including exit.LogExit) saw codes.Unknown instead of Unauthenticated/PermissionDenied. This affects any connector using clientcredentials.Config-based auth via BaseHttpClient, whether hand-rolled or through OAuth2ClientCredentials. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 14 ++++++++++++++ pkg/uhttp/errors_test.go | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index ec719322c..ce6bbfaae 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -10,6 +10,7 @@ import ( "net/url" "strings" + "golang.org/x/oauth2" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -20,6 +21,19 @@ func wrapTransientNetworkError(err error) error { if err == nil { return nil } + + // A failed OAuth2 token exchange (rejected client credentials, wrong + // scope, expired secret, etc.) surfaces here as *oauth2.RetrieveError + // wrapping the token endpoint's real HTTP response — not as a network + // blip. Map its status code the same way a normal API response would + // be mapped, instead of falling through to codes.Unknown: callers + // (e.g. exit.LogExit) rely on that mapping to tell a real auth failure + // apart from an unclassified error. + var retrieveErr *oauth2.RetrieveError + if errors.As(err, &retrieveErr) && retrieveErr.Response != nil { + return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), retrieveErr.Response.Status, err) + } + if errors.Is(err, io.ErrUnexpectedEOF) { return WrapErrors(codes.Unavailable, "unexpected EOF", err) } diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index 2131bd677..e7398b991 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "net" + "net/http" "net/url" "os" "syscall" @@ -11,6 +12,7 @@ import ( "time" "github.com/stretchr/testify/require" + "golang.org/x/oauth2" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -172,6 +174,23 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.Unavailable, wantMsg: "http2 client connection lost", }, + { + name: "oauth2 token exchange rejected (401)", + err: &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusUnauthorized, Status: "401 Unauthorized"}, + Body: []byte(`{"error":"invalid_client"}`), + }, + wantCode: codes.Unauthenticated, + wantMsg: "401 Unauthorized", + }, + { + name: "oauth2 token exchange forbidden (403)", + err: &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusForbidden, Status: "403 Forbidden"}, + }, + wantCode: codes.PermissionDenied, + wantMsg: "403 Forbidden", + }, } for _, tt := range tests { From b876f7e38cc8ec3a040773685e09b97667d0e0a0 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Wed, 26 Aug 2026 16:51:23 -0300 Subject: [PATCH 2/9] Key oauth2 token-exchange error mapping off RFC 6749's error param Mapping GrpcCodeFromHTTPStatus(Response.StatusCode) alone missed two real shapes: some token endpoints report the error param on a 2xx response, and RFC 6749 5.2 makes 400 (not 401) the default status for invalid_client/invalid_grant, which the status-only mapping turned into InvalidArgument instead of Unauthenticated. Consult RetrieveError.ErrorCode first and fall back to the HTTP status only when the server didn't send a recognized error code. Also make the new test cases wrap *oauth2.RetrieveError in *url.Error, the shape http.Client.Do actually produces, so the errors.As unwrap this fix depends on is exercised against its real production shape. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 54 ++++++++++++++++++++++++---- pkg/uhttp/errors_test.go | 78 +++++++++++++++++++++++++++++++++++----- 2 files changed, 117 insertions(+), 15 deletions(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index ce6bbfaae..644ae4eec 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -24,14 +24,24 @@ func wrapTransientNetworkError(err error) error { // A failed OAuth2 token exchange (rejected client credentials, wrong // scope, expired secret, etc.) surfaces here as *oauth2.RetrieveError - // wrapping the token endpoint's real HTTP response — not as a network - // blip. Map its status code the same way a normal API response would - // be mapped, instead of falling through to codes.Unknown: callers - // (e.g. exit.LogExit) rely on that mapping to tell a real auth failure + // wrapping the token endpoint's real response — not as a network blip. + // RFC 6749 §5.2's "error" parameter is the authoritative signal: some + // servers report it on an HTTP 200 (x/oauth2 still treats that as a + // RetrieveError), and the spec's own default status for invalid_client/ + // invalid_grant is 400, which GrpcCodeFromHTTPStatus maps to + // InvalidArgument — not the Unauthenticated/PermissionDenied a bad- + // credentials rejection should produce. Only fall back to the HTTP + // status when the server didn't send a recognized error code. Callers + // (e.g. exit.LogExit) rely on this mapping to tell a real auth failure // apart from an unclassified error. var retrieveErr *oauth2.RetrieveError - if errors.As(err, &retrieveErr) && retrieveErr.Response != nil { - return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), retrieveErr.Response.Status, err) + if errors.As(err, &retrieveErr) { + if code, ok := oauthTokenErrorCode(retrieveErr.ErrorCode); ok { + return WrapErrors(code, oauthTokenErrorMessage(retrieveErr), err) + } + if retrieveErr.Response != nil { + return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), oauthTokenErrorMessage(retrieveErr), err) + } } if errors.Is(err, io.ErrUnexpectedEOF) { @@ -102,6 +112,38 @@ func wrapTransientNetworkError(err error) error { return err } +// oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to +// a grpc code. ok is false when errCode is empty or not one of the values +// the spec defines, signaling the caller to fall back to the HTTP status. +func oauthTokenErrorCode(errCode string) (code codes.Code, ok bool) { + switch errCode { + case "invalid_client", "unauthorized_client": + return codes.Unauthenticated, true + case "access_denied": + return codes.PermissionDenied, true + case "invalid_grant", "invalid_scope", "invalid_request", "unsupported_grant_type", "unsupported_response_type": + return codes.InvalidArgument, true + default: + return codes.Unknown, false + } +} + +// oauthTokenErrorMessage prefers the RFC 6749 error/error_description pair +// the token endpoint sent, since that survives even when the HTTP status +// alone would be misleading (e.g. a 200 response carrying an error body). +func oauthTokenErrorMessage(retrieveErr *oauth2.RetrieveError) string { + switch { + case retrieveErr.ErrorCode != "" && retrieveErr.ErrorDescription != "": + return fmt.Sprintf("%s: %s", retrieveErr.ErrorCode, retrieveErr.ErrorDescription) + case retrieveErr.ErrorCode != "": + return retrieveErr.ErrorCode + case retrieveErr.Response != nil: + return retrieveErr.Response.Status + default: + return "oauth2 token request failed" + } +} + func isHTTP2ClientConnectionLost(err error) bool { return strings.Contains(err.Error(), "http2: client connection lost") } diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index e7398b991..f3c00be28 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -19,6 +19,14 @@ import ( "github.com/conductorone/baton-sdk/pkg/retry" ) +// wrapAsTokenRequestError mirrors the shape wrapTransientNetworkError +// actually receives in production: oauth2.Transport's RoundTrip returns the +// token-source error unwrapped, and http.Client.Do wraps it in *url.Error +// before BaseHttpClient.Do ever sees it. +func wrapAsTokenRequestError(retrieveErr *oauth2.RetrieveError) error { + return &url.Error{Op: "Post", URL: "https://example.com/oauth/token", Err: retrieveErr} +} + func TestWrapTransientNetworkError(t *testing.T) { tests := []struct { name string @@ -175,19 +183,71 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "http2 client connection lost", }, { - name: "oauth2 token exchange rejected (401)", - err: &oauth2.RetrieveError{ - Response: &http.Response{StatusCode: http.StatusUnauthorized, Status: "401 Unauthorized"}, - Body: []byte(`{"error":"invalid_client"}`), - }, + // Production always delivers *oauth2.RetrieveError wrapped in + // *url.Error (http.Client.Do's own wrapping), so this — not a + // bare RetrieveError — is the shape the errors.As unwrap in + // wrapTransientNetworkError actually has to see through. + name: "oauth2 invalid_client (RFC 6749 error param, 401)", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_client", + ErrorDescription: "client authentication failed", + Response: &http.Response{StatusCode: http.StatusUnauthorized, Status: "401 Unauthorized"}, + }), + wantCode: codes.Unauthenticated, + wantMsg: "invalid_client: client authentication failed", + }, + { + // RFC 6749 §5.2 makes 400 the default status for invalid_client/ + // invalid_grant (401 is only a MAY for invalid_client). Relying on + // GrpcCodeFromHTTPStatus(400) alone would produce InvalidArgument + // for exactly the credentials-rejected case this exists to catch; + // the "error" param must take priority over the HTTP status. + name: "oauth2 invalid_client on the RFC default status (400)", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_client", + Response: &http.Response{StatusCode: http.StatusBadRequest, Status: "400 Bad Request"}, + }), + wantCode: codes.Unauthenticated, + wantMsg: "invalid_client", + }, + { + // Some token endpoints report the RFC 6749 error param on a 2xx + // response. GrpcCodeFromHTTPStatus(200) would silently map this to + // Unknown with a misleading "200 OK" message if the error param + // weren't consulted first. + name: "oauth2 error param on a 200 response", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_client", + Response: &http.Response{StatusCode: http.StatusOK, Status: "200 OK"}, + }), wantCode: codes.Unauthenticated, - wantMsg: "401 Unauthorized", + wantMsg: "invalid_client", + }, + { + name: "oauth2 access_denied", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "access_denied", + Response: &http.Response{StatusCode: http.StatusForbidden, Status: "403 Forbidden"}, + }), + wantCode: codes.PermissionDenied, + wantMsg: "access_denied", }, { - name: "oauth2 token exchange forbidden (403)", - err: &oauth2.RetrieveError{ + name: "oauth2 invalid_grant maps to InvalidArgument, not an auth failure", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_grant", + Response: &http.Response{StatusCode: http.StatusBadRequest, Status: "400 Bad Request"}, + }), + wantCode: codes.InvalidArgument, + wantMsg: "invalid_grant", + }, + { + // No RFC 6749 error param at all (a non-compliant or proxy-mangled + // response) falls back to the plain HTTP status. + name: "oauth2 token request rejected with no error param (403)", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ Response: &http.Response{StatusCode: http.StatusForbidden, Status: "403 Forbidden"}, - }, + }), wantCode: codes.PermissionDenied, wantMsg: "403 Forbidden", }, From 32cd78d3a4d373822df1e28bc2ef0291be0cb15a Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Wed, 26 Aug 2026 17:00:33 -0300 Subject: [PATCH 3/9] Fix nonamedreturns lint: drop named returns from oauthTokenErrorCode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every branch returns explicit values, so the names were unused by the function body — nonamedreturns flags them regardless. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index 644ae4eec..d1323f948 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -115,7 +115,7 @@ func wrapTransientNetworkError(err error) error { // oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to // a grpc code. ok is false when errCode is empty or not one of the values // the spec defines, signaling the caller to fall back to the HTTP status. -func oauthTokenErrorCode(errCode string) (code codes.Code, ok bool) { +func oauthTokenErrorCode(errCode string) (codes.Code, bool) { switch errCode { case "invalid_client", "unauthorized_client": return codes.Unauthenticated, true From b2c238db3f6fde8ec64f573c964996d1aef5a224 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Wed, 26 Aug 2026 17:04:06 -0300 Subject: [PATCH 4/9] Correct invalid_grant and unauthorized_client mapping per RFC 6749 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit invalid_grant explicitly covers "resource owner credentials" (a wrong username/password under the password grant), not just an authorization code or refresh token — a genuine credential failure, so it belongs with invalid_client's Unauthenticated rather than InvalidArgument. unauthorized_client per RFC 6749 5.2 is "the authenticated client is not authorized to use this authorization grant type" — the identity was accepted, so PermissionDenied fits better than Unauthenticated. Also add coverage for the two classification-flip cases raised in review: a 429/5xx token-endpoint failure now falls back to codes.Unavailable (and becomes retryable, documented in the PR body), and an unrecognized error param falls back to the HTTP status while keeping the error param in the message. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 22 +++++++++++++--- pkg/uhttp/errors_test.go | 57 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index d1323f948..78cd8417c 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -115,13 +115,29 @@ func wrapTransientNetworkError(err error) error { // oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to // a grpc code. ok is false when errCode is empty or not one of the values // the spec defines, signaling the caller to fall back to the HTTP status. +// +// - invalid_client and unauthorized_client both name the client's +// credential/identity as the problem — RFC 6749 defines invalid_client as +// "Client authentication failed", and unauthorized_client as "The +// authenticated client is not authorized to use this authorization grant +// type" (authenticated, but not entitled) — PermissionDenied, not +// Unauthenticated, since re-presenting a different secret won't fix a +// grant-type mismatch. +// - invalid_grant explicitly covers "resource owner credentials" (a wrong +// username/password under the password grant) alongside an expired or +// revoked authorization code/refresh token — a genuine credential +// failure, so it maps with invalid_client rather than the InvalidArgument +// bucket. +// - access_denied, invalid_scope, invalid_request, unsupported_grant_type, +// and unsupported_response_type describe a malformed or disallowed +// request rather than a rejected identity. func oauthTokenErrorCode(errCode string) (codes.Code, bool) { switch errCode { - case "invalid_client", "unauthorized_client": + case "invalid_client", "invalid_grant": return codes.Unauthenticated, true - case "access_denied": + case "unauthorized_client", "access_denied": return codes.PermissionDenied, true - case "invalid_grant", "invalid_scope", "invalid_request", "unsupported_grant_type", "unsupported_response_type": + case "invalid_scope", "invalid_request", "unsupported_grant_type", "unsupported_response_type": return codes.InvalidArgument, true default: return codes.Unknown, false diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index f3c00be28..d4f0bc1af 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -233,14 +233,41 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "access_denied", }, { - name: "oauth2 invalid_grant maps to InvalidArgument, not an auth failure", + // RFC 6749 §5.2 names "resource owner credentials" (a wrong + // username/password under the password grant) as one of the + // things invalid_grant covers — a genuine credential failure, + // not a malformed request, so it belongs with invalid_client. + name: "oauth2 invalid_grant is a credential failure, not InvalidArgument", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "invalid_grant", Response: &http.Response{StatusCode: http.StatusBadRequest, Status: "400 Bad Request"}, }), - wantCode: codes.InvalidArgument, + wantCode: codes.Unauthenticated, wantMsg: "invalid_grant", }, + { + // RFC 6749 §5.2: "The authenticated client is not authorized to + // use this authorization grant type" — the identity was + // accepted, so PermissionDenied fits better than + // Unauthenticated (re-presenting a different secret can't fix a + // grant-type mismatch). + name: "oauth2 unauthorized_client is authenticated but not entitled", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "unauthorized_client", + Response: &http.Response{StatusCode: http.StatusBadRequest, Status: "400 Bad Request"}, + }), + wantCode: codes.PermissionDenied, + wantMsg: "unauthorized_client", + }, + { + name: "oauth2 invalid_scope is a malformed request, not a credential failure", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_scope", + Response: &http.Response{StatusCode: http.StatusBadRequest, Status: "400 Bad Request"}, + }), + wantCode: codes.InvalidArgument, + wantMsg: "invalid_scope", + }, { // No RFC 6749 error param at all (a non-compliant or proxy-mangled // response) falls back to the plain HTTP status. @@ -251,6 +278,32 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.PermissionDenied, wantMsg: "403 Forbidden", }, + { + // A transient failure at the token endpoint (rate limited or the + // server having trouble) has no RFC 6749 error param to key off, + // so it falls back to the HTTP status like any other API + // response — including retry eligibility: this becomes + // Unavailable, which retry.Retryer.ShouldWaitAndRetry treats as + // retryable, same as a 503 on a normal API call. + name: "oauth2 token endpoint rate limited (429) falls back to status and is retryable", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusTooManyRequests, Status: "429 Too Many Requests"}, + }), + wantCode: codes.Unavailable, + wantMsg: "429 Too Many Requests", + }, + { + // An error code the RFC doesn't define (a non-compliant server, + // or a future extension this package doesn't know about) also + // falls back to the HTTP status rather than being dropped. + name: "oauth2 unrecognized error param falls back to status code, keeps the error param in the message", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "some_vendor_specific_error", + Response: &http.Response{StatusCode: http.StatusBadRequest, Status: "400 Bad Request"}, + }), + wantCode: codes.InvalidArgument, + wantMsg: "some_vendor_specific_error", + }, } for _, tt := range tests { From 536e0beecfe18b0fad34428d923751995eb88340 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Wed, 26 Aug 2026 17:09:18 -0300 Subject: [PATCH 5/9] Trim comments; rationale lives in the PR description, not the code Also fixes a stale doc comment left over from the previous commit that still described unauthorized_client as mapping the same as invalid_client. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 36 ++++++------------------------------ pkg/uhttp/errors_test.go | 39 ++------------------------------------- 2 files changed, 8 insertions(+), 67 deletions(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index 78cd8417c..a17721a07 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -22,18 +22,10 @@ func wrapTransientNetworkError(err error) error { return nil } - // A failed OAuth2 token exchange (rejected client credentials, wrong - // scope, expired secret, etc.) surfaces here as *oauth2.RetrieveError - // wrapping the token endpoint's real response — not as a network blip. - // RFC 6749 §5.2's "error" parameter is the authoritative signal: some - // servers report it on an HTTP 200 (x/oauth2 still treats that as a - // RetrieveError), and the spec's own default status for invalid_client/ - // invalid_grant is 400, which GrpcCodeFromHTTPStatus maps to - // InvalidArgument — not the Unauthenticated/PermissionDenied a bad- - // credentials rejection should produce. Only fall back to the HTTP - // status when the server didn't send a recognized error code. Callers - // (e.g. exit.LogExit) rely on this mapping to tell a real auth failure - // apart from an unclassified error. + // The RFC 6749 §5.2 error param takes priority over the HTTP status: + // some servers report it on a 200, and 400 is the spec default for + // invalid_client/invalid_grant, which GrpcCodeFromHTTPStatus alone + // would otherwise misclassify. var retrieveErr *oauth2.RetrieveError if errors.As(err, &retrieveErr) { if code, ok := oauthTokenErrorCode(retrieveErr.ErrorCode); ok { @@ -113,24 +105,8 @@ func wrapTransientNetworkError(err error) error { } // oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to -// a grpc code. ok is false when errCode is empty or not one of the values -// the spec defines, signaling the caller to fall back to the HTTP status. -// -// - invalid_client and unauthorized_client both name the client's -// credential/identity as the problem — RFC 6749 defines invalid_client as -// "Client authentication failed", and unauthorized_client as "The -// authenticated client is not authorized to use this authorization grant -// type" (authenticated, but not entitled) — PermissionDenied, not -// Unauthenticated, since re-presenting a different secret won't fix a -// grant-type mismatch. -// - invalid_grant explicitly covers "resource owner credentials" (a wrong -// username/password under the password grant) alongside an expired or -// revoked authorization code/refresh token — a genuine credential -// failure, so it maps with invalid_client rather than the InvalidArgument -// bucket. -// - access_denied, invalid_scope, invalid_request, unsupported_grant_type, -// and unsupported_response_type describe a malformed or disallowed -// request rather than a rejected identity. +// a grpc code. ok is false when errCode is empty or unrecognized, signaling +// the caller to fall back to the HTTP status. func oauthTokenErrorCode(errCode string) (codes.Code, bool) { switch errCode { case "invalid_client", "invalid_grant": diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index d4f0bc1af..6ff1f069e 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -19,10 +19,8 @@ import ( "github.com/conductorone/baton-sdk/pkg/retry" ) -// wrapAsTokenRequestError mirrors the shape wrapTransientNetworkError -// actually receives in production: oauth2.Transport's RoundTrip returns the -// token-source error unwrapped, and http.Client.Do wraps it in *url.Error -// before BaseHttpClient.Do ever sees it. +// wrapAsTokenRequestError mirrors the *url.Error wrapping http.Client.Do +// actually produces in production, not a bare *oauth2.RetrieveError. func wrapAsTokenRequestError(retrieveErr *oauth2.RetrieveError) error { return &url.Error{Op: "Post", URL: "https://example.com/oauth/token", Err: retrieveErr} } @@ -183,10 +181,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "http2 client connection lost", }, { - // Production always delivers *oauth2.RetrieveError wrapped in - // *url.Error (http.Client.Do's own wrapping), so this — not a - // bare RetrieveError — is the shape the errors.As unwrap in - // wrapTransientNetworkError actually has to see through. name: "oauth2 invalid_client (RFC 6749 error param, 401)", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "invalid_client", @@ -197,11 +191,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "invalid_client: client authentication failed", }, { - // RFC 6749 §5.2 makes 400 the default status for invalid_client/ - // invalid_grant (401 is only a MAY for invalid_client). Relying on - // GrpcCodeFromHTTPStatus(400) alone would produce InvalidArgument - // for exactly the credentials-rejected case this exists to catch; - // the "error" param must take priority over the HTTP status. name: "oauth2 invalid_client on the RFC default status (400)", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "invalid_client", @@ -211,10 +200,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "invalid_client", }, { - // Some token endpoints report the RFC 6749 error param on a 2xx - // response. GrpcCodeFromHTTPStatus(200) would silently map this to - // Unknown with a misleading "200 OK" message if the error param - // weren't consulted first. name: "oauth2 error param on a 200 response", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "invalid_client", @@ -233,10 +218,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "access_denied", }, { - // RFC 6749 §5.2 names "resource owner credentials" (a wrong - // username/password under the password grant) as one of the - // things invalid_grant covers — a genuine credential failure, - // not a malformed request, so it belongs with invalid_client. name: "oauth2 invalid_grant is a credential failure, not InvalidArgument", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "invalid_grant", @@ -246,11 +227,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "invalid_grant", }, { - // RFC 6749 §5.2: "The authenticated client is not authorized to - // use this authorization grant type" — the identity was - // accepted, so PermissionDenied fits better than - // Unauthenticated (re-presenting a different secret can't fix a - // grant-type mismatch). name: "oauth2 unauthorized_client is authenticated but not entitled", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "unauthorized_client", @@ -269,8 +245,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "invalid_scope", }, { - // No RFC 6749 error param at all (a non-compliant or proxy-mangled - // response) falls back to the plain HTTP status. name: "oauth2 token request rejected with no error param (403)", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ Response: &http.Response{StatusCode: http.StatusForbidden, Status: "403 Forbidden"}, @@ -279,12 +253,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "403 Forbidden", }, { - // A transient failure at the token endpoint (rate limited or the - // server having trouble) has no RFC 6749 error param to key off, - // so it falls back to the HTTP status like any other API - // response — including retry eligibility: this becomes - // Unavailable, which retry.Retryer.ShouldWaitAndRetry treats as - // retryable, same as a 503 on a normal API call. name: "oauth2 token endpoint rate limited (429) falls back to status and is retryable", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ Response: &http.Response{StatusCode: http.StatusTooManyRequests, Status: "429 Too Many Requests"}, @@ -293,9 +261,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantMsg: "429 Too Many Requests", }, { - // An error code the RFC doesn't define (a non-compliant server, - // or a future extension this package doesn't know about) also - // falls back to the HTTP status rather than being dropped. name: "oauth2 unrecognized error param falls back to status code, keeps the error param in the message", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "some_vendor_specific_error", From ee3cec08bc3571af8ab2de5b05b979ad50d9c9a4 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Thu, 27 Aug 2026 10:53:44 -0300 Subject: [PATCH 6/9] Let a transient status win over a recognized error param; make the RetrieveError branch total A 429/5xx token-endpoint response with a recognized RFC 6749 error param (some providers report throttling as invalid_request) was mapping to InvalidArgument and losing retry eligibility. Check the transient status first, regardless of the error param. Also make the errors.As branch total: previously an unmatched error code with a nil Response fell through to the rest of the function, where isHTTP2ClientConnectionLost calls err.Error(), and RetrieveError.Error() dereferences Response.Status unconditionally when ErrorCode is empty -- a nil Response there would panic. The vendored x/oauth2 always populates Response today, so this wasn't a live crash, but nothing in the type's contract guarantees that. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 25 +++++++++++++++++++----- pkg/uhttp/errors_test.go | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index a17721a07..a173bfd4d 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -22,18 +22,26 @@ func wrapTransientNetworkError(err error) error { return nil } - // The RFC 6749 §5.2 error param takes priority over the HTTP status: - // some servers report it on a 200, and 400 is the spec default for - // invalid_client/invalid_grant, which GrpcCodeFromHTTPStatus alone - // would otherwise misclassify. + // A transient token-endpoint status (429/5xx) stays retryable even if + // the body also carries a recognized RFC 6749 error param; otherwise + // the error param takes priority over the HTTP status, since some + // servers report it on a 200 and 400 is the spec default for + // invalid_client/invalid_grant, both of which GrpcCodeFromHTTPStatus + // alone would misclassify. This branch is total once errors.As matches, + // so a RetrieveError never reaches the err.Error() calls below it. var retrieveErr *oauth2.RetrieveError if errors.As(err, &retrieveErr) { + if retrieveErr.Response != nil && isTransientHTTPStatus(retrieveErr.Response.StatusCode) { + return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), retrieveErr.Response.Status, err) + } if code, ok := oauthTokenErrorCode(retrieveErr.ErrorCode); ok { return WrapErrors(code, oauthTokenErrorMessage(retrieveErr), err) } + code := codes.Unknown if retrieveErr.Response != nil { - return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), oauthTokenErrorMessage(retrieveErr), err) + code = GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode) } + return WrapErrors(code, oauthTokenErrorMessage(retrieveErr), err) } if errors.Is(err, io.ErrUnexpectedEOF) { @@ -104,6 +112,13 @@ func wrapTransientNetworkError(err error) error { return err } +// isTransientHTTPStatus mirrors the statuses GrpcCodeFromHTTPStatus maps to +// codes.Unavailable, so a transient token-endpoint failure stays retryable +// regardless of what error param the body also carries. +func isTransientHTTPStatus(statusCode int) bool { + return statusCode == http.StatusTooManyRequests || statusCode >= 500 +} + // oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to // a grpc code. ok is false when errCode is empty or unrecognized, signaling // the caller to fall back to the HTTP status. diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index 6ff1f069e..cdf168c06 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -269,6 +269,23 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.InvalidArgument, wantMsg: "some_vendor_specific_error", }, + { + name: "oauth2 transient status wins over a recognized error param", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_request", + Response: &http.Response{StatusCode: http.StatusTooManyRequests, Status: "429 Too Many Requests"}, + }), + wantCode: codes.Unavailable, + wantMsg: "429 Too Many Requests", + }, + { + name: "oauth2 retrieve error with no error param and no response still maps, not falls through", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "", + }), + wantCode: codes.Unknown, + wantMsg: "oauth2 token request failed", + }, } for _, tt := range tests { @@ -323,6 +340,31 @@ func TestWrapTransientNetworkError_NXDOMAINIsTerminal(t *testing.T) { "a temporary resolver failure must still be retried") } +func TestWrapTransientNetworkError_OAuthTokenEndpointTransientIsRetried(t *testing.T) { + newRetryer := func() *retry.Retryer { + return retry.NewRetryer(t.Context(), retry.RetryConfig{ + MaxAttempts: 3, + InitialDelay: time.Millisecond, + MaxDelay: time.Millisecond, + }) + } + + rateLimited := wrapTransientNetworkError(wrapAsTokenRequestError(&oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusTooManyRequests, Status: "429 Too Many Requests"}, + })) + require.Equal(t, codes.Unavailable, status.Code(rateLimited)) + require.True(t, newRetryer().ShouldWaitAndRetry(t.Context(), rateLimited), + "a rate-limited token endpoint must still be retried") + + rejected := wrapTransientNetworkError(wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_client", + Response: &http.Response{StatusCode: http.StatusUnauthorized, Status: "401 Unauthorized"}, + })) + require.Equal(t, codes.Unauthenticated, status.Code(rejected)) + require.False(t, newRetryer().ShouldWaitAndRetry(t.Context(), rejected), + "rejected credentials must not be retried") +} + func TestWrapTransientNetworkError_LeavesNonTransientAlone(t *testing.T) { err := fmt.Errorf("something went wrong") got := wrapTransientNetworkError(err) From 85a3c28ac8b17d2b479ae82d57fe40c26cd964c7 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Thu, 27 Aug 2026 11:06:33 -0300 Subject: [PATCH 7/9] Make isTransientHTTPStatus exact; keep error_description on transient msg isTransientHTTPStatus approximated GrpcCodeFromHTTPStatus's Unavailable set with a numeric range, which wrongly included 501 (explicitly mapped to Unimplemented, not Unavailable). Consult the mapping directly so the two can't drift. The transient-status message also dropped ErrorDescription, which usually carries the most actionable detail (e.g. a rate-limit retry hint). Keep the status text (it explains the Unavailable classification) but append the description when the server sent one. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 21 ++++++++++++++++----- pkg/uhttp/errors_test.go | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index a173bfd4d..4b929491c 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -32,7 +32,7 @@ func wrapTransientNetworkError(err error) error { var retrieveErr *oauth2.RetrieveError if errors.As(err, &retrieveErr) { if retrieveErr.Response != nil && isTransientHTTPStatus(retrieveErr.Response.StatusCode) { - return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), retrieveErr.Response.Status, err) + return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), transientOAuthTokenMessage(retrieveErr), err) } if code, ok := oauthTokenErrorCode(retrieveErr.ErrorCode); ok { return WrapErrors(code, oauthTokenErrorMessage(retrieveErr), err) @@ -112,11 +112,22 @@ func wrapTransientNetworkError(err error) error { return err } -// isTransientHTTPStatus mirrors the statuses GrpcCodeFromHTTPStatus maps to -// codes.Unavailable, so a transient token-endpoint failure stays retryable -// regardless of what error param the body also carries. +// isTransientHTTPStatus reports whether GrpcCodeFromHTTPStatus maps +// statusCode to codes.Unavailable, so a transient token-endpoint failure +// stays retryable regardless of what error param the body also carries. func isTransientHTTPStatus(statusCode int) bool { - return statusCode == http.StatusTooManyRequests || statusCode >= 500 + return GrpcCodeFromHTTPStatus(statusCode) == codes.Unavailable +} + +// transientOAuthTokenMessage leads with the HTTP status, since that's what +// drove the Unavailable classification, but keeps ErrorDescription when the +// server sent one alongside it (e.g. a rate-limit message). +func transientOAuthTokenMessage(retrieveErr *oauth2.RetrieveError) string { + msg := retrieveErr.Response.Status + if retrieveErr.ErrorDescription != "" { + msg = fmt.Sprintf("%s: %s", msg, retrieveErr.ErrorDescription) + } + return msg } // oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index cdf168c06..7b040f0c2 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -286,6 +286,28 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.Unknown, wantMsg: "oauth2 token request failed", }, + { + name: "oauth2 transient status keeps the error_description, not just the status", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_request", + ErrorDescription: "rate limit exceeded, retry after 3600 seconds", + Response: &http.Response{StatusCode: http.StatusTooManyRequests, Status: "429 Too Many Requests"}, + }), + wantCode: codes.Unavailable, + wantMsg: "429 Too Many Requests: rate limit exceeded, retry after 3600 seconds", + }, + { + // 501 is Unimplemented, not part of GrpcCodeFromHTTPStatus's + // Unavailable set, even though it's >= 500 — isTransientHTTPStatus + // must consult the mapping, not approximate it with a numeric range. + name: "oauth2 501 is not transient (Unimplemented, not Unavailable)", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_client", + Response: &http.Response{StatusCode: http.StatusNotImplemented, Status: "501 Not Implemented"}, + }), + wantCode: codes.Unauthenticated, + wantMsg: "invalid_client", + }, } for _, tt := range tests { From d7ff8bc238d1a49a183e77c73972ab228a97eb90 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Thu, 27 Aug 2026 11:39:48 -0300 Subject: [PATCH 8/9] Widen transient set to DeadlineExceeded; attach rate-limit details; correct an overstated panic claim isTransientHTTPStatus only checked for Unavailable, but 408 maps to DeadlineExceeded, which retry.Retryer also treats as retryable -- the same asymmetry the transient-status-first ordering exists to prevent. Now checks both codes. The transient branch was building its own status manually, discarding any rate-limit detail retrieveErr.Response's real headers carry. wrapTransientOAuthTokenError now extracts and attaches it the same way WrapErrorsWithRateLimitInfo does for normal API responses, so retry.Retryer can compute rate-limit-aware backoff for a throttled token endpoint too. Reverted the "don't join err" change from the previous commit: verified (not guessed) that it wasn't preventing a real panic. RetrieveError's panic on a nil Response only ever occurs while it's being formatted by a %s/%v verb -- inside url.Error.Error()'s own fmt.Sprintf call -- and fmt recovers panics from Stringer/error methods it formats internally, substituting "%!s(PANIC=...)" instead of propagating. Confirmed this holds through errors.Join too. Restored the simpler WrapErrors call and the errors.Is/As chain to err that not joining it had discarded. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors.go | 33 +++++++++++++++------ pkg/uhttp/errors_test.go | 62 ++++++++++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/pkg/uhttp/errors.go b/pkg/uhttp/errors.go index 4b929491c..005b68a8e 100644 --- a/pkg/uhttp/errors.go +++ b/pkg/uhttp/errors.go @@ -13,6 +13,8 @@ import ( "golang.org/x/oauth2" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + + "github.com/conductorone/baton-sdk/pkg/ratelimit" ) // wrapTransientNetworkError mirrors Baton HTTP retry classification for callers @@ -28,11 +30,11 @@ func wrapTransientNetworkError(err error) error { // servers report it on a 200 and 400 is the spec default for // invalid_client/invalid_grant, both of which GrpcCodeFromHTTPStatus // alone would misclassify. This branch is total once errors.As matches, - // so a RetrieveError never reaches the err.Error() calls below it. + // so a RetrieveError is never run through the network-error checks below. var retrieveErr *oauth2.RetrieveError if errors.As(err, &retrieveErr) { if retrieveErr.Response != nil && isTransientHTTPStatus(retrieveErr.Response.StatusCode) { - return WrapErrors(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), transientOAuthTokenMessage(retrieveErr), err) + return wrapTransientOAuthTokenError(retrieveErr, err) } if code, ok := oauthTokenErrorCode(retrieveErr.ErrorCode); ok { return WrapErrors(code, oauthTokenErrorMessage(retrieveErr), err) @@ -113,21 +115,34 @@ func wrapTransientNetworkError(err error) error { } // isTransientHTTPStatus reports whether GrpcCodeFromHTTPStatus maps -// statusCode to codes.Unavailable, so a transient token-endpoint failure +// statusCode to a code retry.Retryer.ShouldWaitAndRetry treats as retryable +// (Unavailable or DeadlineExceeded), so a transient token-endpoint failure // stays retryable regardless of what error param the body also carries. func isTransientHTTPStatus(statusCode int) bool { - return GrpcCodeFromHTTPStatus(statusCode) == codes.Unavailable + switch GrpcCodeFromHTTPStatus(statusCode) { + case codes.Unavailable, codes.DeadlineExceeded: + return true + default: + return false + } } -// transientOAuthTokenMessage leads with the HTTP status, since that's what -// drove the Unavailable classification, but keeps ErrorDescription when the -// server sent one alongside it (e.g. a rate-limit message). -func transientOAuthTokenMessage(retrieveErr *oauth2.RetrieveError) string { +// wrapTransientOAuthTokenError mirrors WrapErrorsWithRateLimitInfo's detail +// attachment (retry.Retryer reads it for rate-limit-aware backoff), while +// keeping ErrorDescription in the message the way oauthTokenErrorMessage +// does elsewhere in this file. +func wrapTransientOAuthTokenError(retrieveErr *oauth2.RetrieveError, err error) error { msg := retrieveErr.Response.Status if retrieveErr.ErrorDescription != "" { msg = fmt.Sprintf("%s: %s", msg, retrieveErr.ErrorDescription) } - return msg + st := status.New(GrpcCodeFromHTTPStatus(retrieveErr.Response.StatusCode), msg) + if description, rlErr := ratelimit.ExtractRateLimitData(retrieveErr.Response.StatusCode, &retrieveErr.Response.Header); rlErr == nil { + if withDetails, detailsErr := st.WithDetails(description); detailsErr == nil { + st = withDetails + } + } + return errors.Join(st.Err(), err) } // oauthTokenErrorCode maps an RFC 6749 §5.2 token-error "error" parameter to diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index 7b040f0c2..52321fcea 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -16,6 +16,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/retry" ) @@ -269,6 +270,14 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.InvalidArgument, wantMsg: "some_vendor_specific_error", }, + { + name: "oauth2 retrieve error with no error param and no response still maps, not falls through", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "", + }), + wantCode: codes.Unknown, + wantMsg: "oauth2 token request failed", + }, { name: "oauth2 transient status wins over a recognized error param", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ @@ -278,14 +287,6 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.Unavailable, wantMsg: "429 Too Many Requests", }, - { - name: "oauth2 retrieve error with no error param and no response still maps, not falls through", - err: wrapAsTokenRequestError(&oauth2.RetrieveError{ - ErrorCode: "", - }), - wantCode: codes.Unknown, - wantMsg: "oauth2 token request failed", - }, { name: "oauth2 transient status keeps the error_description, not just the status", err: wrapAsTokenRequestError(&oauth2.RetrieveError{ @@ -308,6 +309,19 @@ func TestWrapTransientNetworkError(t *testing.T) { wantCode: codes.Unauthenticated, wantMsg: "invalid_client", }, + { + // 408 maps to DeadlineExceeded, which retry.Retryer also treats + // as retryable — isTransientHTTPStatus must catch this too, not + // just Unavailable, or the same status flips outcome depending + // on whether an error param happens to be present. + name: "oauth2 408 (DeadlineExceeded) is also transient", + err: wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_request", + Response: &http.Response{StatusCode: http.StatusRequestTimeout, Status: "408 Request Timeout"}, + }), + wantCode: codes.DeadlineExceeded, + wantMsg: "408 Request Timeout", + }, } for _, tt := range tests { @@ -322,6 +336,38 @@ func TestWrapTransientNetworkError(t *testing.T) { } } +func TestWrapTransientNetworkError_OAuthTransientAttachesRateLimitDetails(t *testing.T) { + header := http.Header{} + header.Set("Retry-After", "120") + got := wrapTransientNetworkError(wrapAsTokenRequestError(&oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusTooManyRequests, Status: "429 Too Many Requests", Header: header}, + })) + + st, ok := status.FromError(got) + require.True(t, ok) + require.Equal(t, codes.Unavailable, st.Code()) + + var found *v2.RateLimitDescription + for _, detail := range st.Details() { + if rl, ok := detail.(*v2.RateLimitDescription); ok { + found = rl + } + } + require.NotNil(t, found, "expected a RateLimitDescription detail from the Retry-After header") +} + +func TestWrapTransientNetworkError_OAuthNilResponseDoesNotPanicOnFormat(t *testing.T) { + got := wrapTransientNetworkError(wrapAsTokenRequestError(&oauth2.RetrieveError{})) + + require.NotPanics(t, func() { + _ = got.Error() + }) + st, ok := status.FromError(got) + require.True(t, ok) + require.Equal(t, codes.Unknown, st.Code()) + require.Contains(t, st.Message(), "oauth2 token request failed") +} + // NXDOMAIN is the one classification here that is deliberately terminal: a // hostname that does not resolve is a misconfiguration, so retrying it burns // the retry budget on every action and preserving the sync hides the cause. From 75094c6fb970241a1fe41b03914a8172e005a098 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Thu, 27 Aug 2026 12:03:27 -0300 Subject: [PATCH 9/9] Add retry-liveness coverage for the DeadlineExceeded transient arm TestWrapTransientNetworkError_OAuthTokenEndpointTransientIsRetried covered Unavailable (429) and terminal (401) but not the DeadlineExceeded arm isTransientHTTPStatus gained. Assert retry.Retryer.ShouldWaitAndRetry against a 408 too. Co-Authored-By: Claude Sonnet 5 --- pkg/uhttp/errors_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/uhttp/errors_test.go b/pkg/uhttp/errors_test.go index 52321fcea..5a4a31d79 100644 --- a/pkg/uhttp/errors_test.go +++ b/pkg/uhttp/errors_test.go @@ -424,6 +424,14 @@ func TestWrapTransientNetworkError_OAuthTokenEndpointTransientIsRetried(t *testi require.True(t, newRetryer().ShouldWaitAndRetry(t.Context(), rateLimited), "a rate-limited token endpoint must still be retried") + timedOut := wrapTransientNetworkError(wrapAsTokenRequestError(&oauth2.RetrieveError{ + ErrorCode: "invalid_request", + Response: &http.Response{StatusCode: http.StatusRequestTimeout, Status: "408 Request Timeout"}, + })) + require.Equal(t, codes.DeadlineExceeded, status.Code(timedOut)) + require.True(t, newRetryer().ShouldWaitAndRetry(t.Context(), timedOut), + "a token endpoint timeout must still be retried") + rejected := wrapTransientNetworkError(wrapAsTokenRequestError(&oauth2.RetrieveError{ ErrorCode: "invalid_client", Response: &http.Response{StatusCode: http.StatusUnauthorized, Status: "401 Unauthorized"},