mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Treat invalid_grant refresh failures as permanent (#39637)
## Why OAuth token endpoints can report an unusable refresh token with the standard `invalid_grant` error instead of a legacy expired, reused, or revoked subtype. ## What changed - Classify `400 Bad Request` responses with an `invalid_grant` error code as permanent refresh failures, preserving the generic failure reason. - Cache that failure so subsequent refresh attempts do not repeat the request. - Keep other `400 Bad Request` errors transient and retryable. ## Testing Added refresh tests covering terminal `invalid_grant` responses and retryable `invalid_request` responses. GitOrigin-RevId: 513d34b514a7e4118a70c76e9d0fe779c006d5d8
This commit is contained in:
@@ -1580,8 +1580,19 @@ async fn request_chatgpt_token_refresh(
|
||||
} else {
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
tracing::error!("Failed to refresh token: {status}: {body}");
|
||||
let failed = classify_refresh_token_failure(&body);
|
||||
if status == StatusCode::UNAUTHORIZED || failed.reason != RefreshTokenFailedReason::Other {
|
||||
let code = extract_refresh_token_error_code(&body);
|
||||
// RFC 6749 reports an unusable refresh token as invalid_grant without preserving
|
||||
// the legacy expired/reused/revoked subtype. Keep it terminal with the generic reason.
|
||||
let is_invalid_grant_bad_request = status == StatusCode::BAD_REQUEST
|
||||
&& code
|
||||
.as_deref()
|
||||
.is_some_and(|code| code.eq_ignore_ascii_case("invalid_grant"));
|
||||
let failed =
|
||||
classify_refresh_token_failure(code.as_deref(), &body, is_invalid_grant_bad_request);
|
||||
if status == StatusCode::UNAUTHORIZED
|
||||
|| failed.reason != RefreshTokenFailedReason::Other
|
||||
|| is_invalid_grant_bad_request
|
||||
{
|
||||
Err(RefreshTokenError::Permanent(failed))
|
||||
} else {
|
||||
let message = try_parse_error_message(&body);
|
||||
@@ -1592,10 +1603,12 @@ async fn request_chatgpt_token_refresh(
|
||||
}
|
||||
}
|
||||
|
||||
fn classify_refresh_token_failure(body: &str) -> RefreshTokenFailedError {
|
||||
let code = extract_refresh_token_error_code(body);
|
||||
|
||||
let normalized_code = code.as_deref().map(str::to_ascii_lowercase);
|
||||
fn classify_refresh_token_failure(
|
||||
code: Option<&str>,
|
||||
body: &str,
|
||||
is_invalid_grant_bad_request: bool,
|
||||
) -> RefreshTokenFailedError {
|
||||
let normalized_code = code.map(str::to_ascii_lowercase);
|
||||
let reason = match normalized_code.as_deref() {
|
||||
Some("refresh_token_expired") => RefreshTokenFailedReason::Expired,
|
||||
Some("refresh_token_reused") => RefreshTokenFailedReason::Exhausted,
|
||||
@@ -1603,7 +1616,7 @@ fn classify_refresh_token_failure(body: &str) -> RefreshTokenFailedError {
|
||||
_ => RefreshTokenFailedReason::Other,
|
||||
};
|
||||
|
||||
if reason == RefreshTokenFailedReason::Other {
|
||||
if reason == RefreshTokenFailedReason::Other && !is_invalid_grant_bad_request {
|
||||
tracing::warn!(
|
||||
backend_code = normalized_code.as_deref(),
|
||||
backend_body = body,
|
||||
|
||||
@@ -986,6 +986,136 @@ async fn refresh_token_does_not_retry_after_bad_request_reused_failure() -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_does_not_retry_after_standard_invalid_grant_failure() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(ResponseTemplate::new(400).set_body_json(json!({
|
||||
"error": "invalid_grant"
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let ctx = RefreshTokenTestContext::new(&server).await?;
|
||||
let initial_last_refresh = Utc::now() - Duration::days(1);
|
||||
let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN);
|
||||
let initial_auth = AuthDotJson {
|
||||
auth_mode: Some(AuthMode::Chatgpt),
|
||||
openai_api_key: None,
|
||||
tokens: Some(initial_tokens.clone()),
|
||||
last_refresh: Some(initial_last_refresh),
|
||||
agent_identity: None,
|
||||
personal_access_token: None,
|
||||
bedrock_api_key: None,
|
||||
};
|
||||
ctx.write_auth(&initial_auth).await?;
|
||||
|
||||
let first_err = ctx
|
||||
.auth_manager
|
||||
.refresh_token()
|
||||
.await
|
||||
.err()
|
||||
.context("first refresh should fail")?;
|
||||
assert_eq!(
|
||||
first_err.failed_reason(),
|
||||
Some(RefreshTokenFailedReason::Other)
|
||||
);
|
||||
|
||||
let second_err = ctx
|
||||
.auth_manager
|
||||
.refresh_token()
|
||||
.await
|
||||
.err()
|
||||
.context("second refresh should fail without retrying")?;
|
||||
assert_eq!(
|
||||
second_err.failed_reason(),
|
||||
Some(RefreshTokenFailedReason::Other)
|
||||
);
|
||||
|
||||
let stored = ctx.load_auth()?;
|
||||
assert_eq!(stored, initial_auth);
|
||||
let cached_auth = ctx
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.context("auth should remain cached")?;
|
||||
let cached = cached_auth
|
||||
.get_token_data()
|
||||
.context("token data should remain cached")?;
|
||||
assert_eq!(cached, initial_tokens);
|
||||
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_does_not_cache_other_bad_request_failure() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(ResponseTemplate::new(400).set_body_json(json!({
|
||||
"error": "invalid_request"
|
||||
})))
|
||||
.expect(2)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let ctx = RefreshTokenTestContext::new(&server).await?;
|
||||
let initial_last_refresh = Utc::now() - Duration::days(1);
|
||||
let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN);
|
||||
let initial_auth = AuthDotJson {
|
||||
auth_mode: Some(AuthMode::Chatgpt),
|
||||
openai_api_key: None,
|
||||
tokens: Some(initial_tokens.clone()),
|
||||
last_refresh: Some(initial_last_refresh),
|
||||
agent_identity: None,
|
||||
personal_access_token: None,
|
||||
bedrock_api_key: None,
|
||||
};
|
||||
ctx.write_auth(&initial_auth).await?;
|
||||
|
||||
let first_err = ctx
|
||||
.auth_manager
|
||||
.refresh_token()
|
||||
.await
|
||||
.err()
|
||||
.context("first refresh should fail")?;
|
||||
assert_eq!(first_err.failed_reason(), None);
|
||||
assert!(matches!(first_err, RefreshTokenError::Transient(_)));
|
||||
|
||||
let second_err = ctx
|
||||
.auth_manager
|
||||
.refresh_token()
|
||||
.await
|
||||
.err()
|
||||
.context("second refresh should retry and fail")?;
|
||||
assert_eq!(second_err.failed_reason(), None);
|
||||
assert!(matches!(second_err, RefreshTokenError::Transient(_)));
|
||||
|
||||
let stored = ctx.load_auth()?;
|
||||
assert_eq!(stored, initial_auth);
|
||||
let cached_auth = ctx
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.context("auth should remain cached")?;
|
||||
let cached = cached_auth
|
||||
.get_token_data()
|
||||
.context("token data should remain cached")?;
|
||||
assert_eq!(cached, initial_tokens);
|
||||
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_reloads_changed_auth_after_permanent_failure() -> Result<()> {
|
||||
|
||||
Reference in New Issue
Block a user