Skip to content

fix(oauth): clear refresh marker on connection failures - #664

Merged
Destynova2 merged 1 commit into
mainfrom
fix/oauth-refresh-connect-wedge
Oct 28, 2026
Merged

Destynova2 merged 1 commit into
mainfrom
fix/oauth-refresh-connect-wedge

Conversation

@Destynova2

Copy link
Copy Markdown
Contributor

What

A refresh that fails before the TCP/TLS connection is established never reaches the issuer, so the stored refresh token is neither spent nor rotated. Yet refresh_token left the .refresh.pending uncertainty marker in place on any error returned by do_refresh:

self.token_store.begin_refresh(&existing_token)?;      // writes <provider>.refresh.pending
let response = self.do_refresh(&existing_token).await?; // transport error propagated as-is

On the next tick, begin_oauth_refresh finds the marker's fingerprint equal to the current refresh token and refuses:

Previous OAuth refresh outcome is unknown; replace credentials or authenticate again

So a single transient connect failure (brief outage, DNS hiccup, daemon waking from sleep) wedged every subsequent refresh permanently, until a manual grob connect --force-reauth — even though the refresh token was still valid. The existing non-success handling only clears the marker on an explicit 400/401/403, which the connection-never-opened case never reaches.

Change

Clear the marker when do_refresh fails with a reqwest connection-phase error (reqwest::Error::is_connect()): the request provably never left the client, so the next tick can safely retry the still-valid token.

Timeouts and post-send/read failures intentionally keep the marker — there the outcome is genuinely unknown, and re-auth is the correct conservative answer.

Test

connect_failure_does_not_wedge_future_refreshes points a store-backed client at http://127.0.0.1:1 (no listener → connect refused) and asserts that two consecutive refreshes both surface the connect error, and neither returns the "outcome is unknown" wedge. Before the fix, the second attempt wedged.

Fixes #663

@Destynova2
Destynova2 enabled auto-merge (squash) October 28, 2026 14:38
@Destynova2
Destynova2 force-pushed the fix/oauth-refresh-connect-wedge branch from af50cb9 to 698eaf3 Compare October 28, 2026 15:51
A refresh that fails before the TCP/TLS connection is established never
reaches the issuer, so the stored refresh token is neither spent nor
rotated. refresh_token left the .refresh.pending uncertainty marker in
place on any error from do_refresh, so one transient connect failure
wedged every later refresh with "Previous OAuth refresh outcome is
unknown" until a manual re-auth, even though the refresh token was still
valid.

Clear the marker when do_refresh fails with a reqwest connection-phase
error (is_connect()); timeouts and post-send failures still keep it,
since there the outcome is genuinely unknown.

Fixes #663
@Destynova2
Destynova2 force-pushed the fix/oauth-refresh-connect-wedge branch from 698eaf3 to a1b31e8 Compare October 28, 2026 16:11
@Destynova2
Destynova2 merged commit 57efbac into main Oct 28, 2026
41 checks passed
@Destynova2
Destynova2 deleted the fix/oauth-refresh-connect-wedge branch October 28, 2026 16:22
Destynova2 added a commit that referenced this pull request Oct 28, 2026
## 🤖 New release

* `grob`: 0.36.128 -> 0.36.129

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.36.129](v0.36.128...v0.36.129)
- 2026-10-28

### Fixed

- *(oauth)* clear refresh marker on connection failures
([#664](#664))
- *(deps)* clear RUSTSEC-2026-0385 and RUSTSEC-2026-0387
([#665](#665))

### Other

- *(deps)* update cargo minor updates
([#661](#661))
- *(deps)* update docker.io/grafana/otel-lgtm docker tag to v0.38.1
([#662](#662))
- *(deps)* update docker.io/curlimages/curl docker tag to v8.23.0
([#649](#649))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
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.

OAuth refresh wedges permanently after a connection-level failure (stale .refresh.pending marker)

1 participant