Skip to content

dpop_oauth2/dpop_grpc: retry transient token exchange failures and cl… - #14

Open
kans wants to merge 4 commits into
mainfrom
mattkaniaris/ce-1113-retry-transient-failures-in-the-gcp-oauth-token-exchange
Open

dpop_oauth2/dpop_grpc: retry transient token exchange failures and cl…#14
kans wants to merge 4 commits into
mainfrom
mattkaniaris/ce-1113-retry-transient-failures-in-the-gcp-oauth-token-exchange

Conversation

@kans

@kans kans commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements both remediation layers from CE-1113: the DPoP token exchange currently has no retry, so a single 500/503, connection reset, or timeout on the token POST kills the calling action (~30–40/day baseline in prod, plus deploy-window 503 bursts).

Layer 1 — retry inside the token source (dpop_oauth2)

  • Token() retries transient failures (5xx, 429, transport errors, timeouts) with capped exponential backoff + jitter: 3 attempts total by default (500ms initial, 2s cap), fitting the existing 30s Token() budget.
  • The retry re-enters tryToken from the top, so every attempt signs a fresh DPoP proof and client assertion — proof jti may be single-use, so an identical request is never replayed. This is why the retry lives here and not in an injected HTTP client.
  • OAuth protocol errors from the response body (invalid_client, disabled credential, other 4xx) are definitive and never retried.
  • Composes with the existing use_dpop_nonce challenge retry: challenges resolve within an attempt; transient retries reuse the cached nonce.
  • Tunable via new WithRetryConfig option (MaxAttempts: 1 disables retries).

Layer 2 — classify what escapes (dpop_grpc)

  • New ErrTokenRequestTransient sentinel + dpop_oauth2.IsTransient(err). Transient errors still match ErrTokenRequestFailed, so existing callers are unaffected.
  • DPoPCredentials.GetRequestMetadata now maps token failures to gRPC statuses: transient → codes.Unavailable (picked up by baton-sdk pkg/retry, which retries Unavailable/DeadlineExceeded — unlimited attempts with backoff in the sync loop), definitive → codes.Unauthenticated (fail fast).
  • grpc-go passes status errors from per-RPC credentials through unflattened (verified in v1.70; neither code is on the gRFC A54 restricted list), so the classification survives to the caller instead of being squashed to Unauthenticated.
  • Per the ticket's anti-goal, pkg/retry in baton-sdk is untouched: a genuinely disabled credential still fails fast rather than spinning in an unlimited retryer.

Notes

  • dpop_grpc now requires the dpop_oauth2 module (require + local replace, same pattern as the existing core-module dependency). make tag tags all modules with the same version, so both release together; baton-sdk and c1 pick this up via dependency bump.
  • The GCP connector fix originally scoped in the ticket (retrying HTTP client on Google's token-exchange context) lives in the connector repo, not here.

Testing

  • Retry-to-success on 5xx, asserting each attempt carried a distinct proof jti.
  • Retry exhaustion (returns transient error after MaxAttempts), no-retry on invalid_client (single call, classified definitive), 429/transport-error/timeout classification, nonce-challenge interplay.
  • dpop_grpc: table-driven status-code mapping tests, plus an end-to-end test running a real token source against a persistent-503 endpoint through bufconn, asserting codes.Unavailable surfaces on the RPC.
  • All tests pass in the core module, dpop_oauth2, and dpop_grpc; gofmt/go vet clean.

…assify what escapes (CE-1113)

Retry 5xx/429, transport errors, and timeouts in the token source with
capped exponential backoff + jitter (3 attempts by default), signing a
fresh DPoP proof and client assertion on every attempt. OAuth protocol
errors are never retried.

Failures that escape are split transient vs definitive: DPoPCredentials
maps transient to codes.Unavailable (retryable by callers) and
definitive to codes.Unauthenticated (fail fast).

Co-authored-by: Cursor <cursoragent@cursor.com>
@linear-code

linear-code Bot commented Aug 28, 2026

Copy link
Copy Markdown

CE-1113

kans and others added 3 commits August 28, 2026 17:14
…hness

- Add a uuid jti to the client assertion: second-precision timestamps plus
  deterministic Ed25519 signatures meant fast retries re-sent a byte-identical
  assertion.
- Carry the nonce learned from a use_dpop_nonce challenge across outer retry
  attempts so bare consumers (no NonceStore) are not re-challenged per retry.
- Stop classifying context.Canceled as transient; preserve the transport error
  chain so callers can inspect the underlying cause.
- Document in dpop_grpc go.mod that the dpop_oauth2 require must be bumped to
  the co-released tag (the local replace does not apply to consumers).

Co-authored-by: Cursor <cursoragent@cursor.com>
…w round 2)

- A caller cancel landing during retry backoff no longer surfaces the previous
  attempt's transient error; it returns a non-transient error carrying
  context.Canceled. Deadline expiry mid-backoff still surfaces the last
  transient failure.
- Detect cancellation on the in-flight POST via ctx.Err() as well as the
  returned error: with context.WithCancelCause, http.Client.Do returns the
  cause, which need not match context.Canceled.
- Guard make tag: refuse to tag unless dpop_grpc's dpop_oauth2 require matches
  the tag, turning a silently broken published module into a release-time error.

Co-authored-by: Cursor <cursoragent@cursor.com>
…iew round 3)

- The tag guard now validates the dpop_oauth2 require in dpop_grpc/go.mod at
  HEAD (what git tag actually tags) with exact field matching, instead of
  regex-grepping the working tree, which an uncommitted bump could defeat.
- The mid-backoff cancel path wraps context.Cause so a WithCancelCause cause
  survives in the error chain, matching the in-flight request path.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant