diff --git a/Cargo.lock b/Cargo.lock index d772798..8f13350 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4814,6 +4814,7 @@ dependencies = [ "base64 0.23.1", "chrono", "html-to-markdown-rs", + "http", "open", "reqwest", "serde", diff --git a/Cargo.toml b/Cargo.toml index 7ee1b3c..93db5fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,6 +55,9 @@ tokio-util = "0.7" croner = "3" tabled = "0.17" reqwest = { version = "0.13", features = ["json", "multipart", "form", "query"] } +# Pinned to the major reqwest 0.13 re-exports, so `http::Response` round-trips +# through `reqwest::Response` (used by the Gmail retry path to buffer a body). +http = "1" quick-xml = { version = "0.41", features = ["serialize"] } tokio-tungstenite = { version = "0.29", features = ["native-tls"] } futures-util = "0.3" diff --git a/crates/void-gmail/Cargo.toml b/crates/void-gmail/Cargo.toml index a5b48e2..4f20d3e 100644 --- a/crates/void-gmail/Cargo.toml +++ b/crates/void-gmail/Cargo.toml @@ -16,6 +16,7 @@ anyhow = { workspace = true } thiserror = { workspace = true } async-trait = { workspace = true } reqwest = { workspace = true } +http = { workspace = true } uuid = { workspace = true } chrono = { workspace = true } tokio-util = { workspace = true } diff --git a/crates/void-gmail/src/api/client.rs b/crates/void-gmail/src/api/client.rs index 368a123..011911f 100644 --- a/crates/void-gmail/src/api/client.rs +++ b/crates/void-gmail/src/api/client.rs @@ -3,6 +3,8 @@ use std::time::Duration; use crate::error::GmailError; use tracing::{debug, info}; +use super::retry::{RetryPolicy, SendRetrying}; + use super::types::{ AttachmentResponse, DraftListResponse, GmailDraft, GmailMessage, GmailProfile, GmailThread, HistoryListResponse, HistoryRecord, LabelListResponse, MessageListResponse, SendAsAlias, @@ -26,6 +28,7 @@ pub struct GmailApiClient { http: reqwest::Client, access_token: String, base_url: String, + retry: RetryPolicy, } impl GmailApiClient { @@ -34,6 +37,7 @@ impl GmailApiClient { http: build_http_client(), access_token: access_token.to_string(), base_url: DEFAULT_BASE_URL.to_string(), + retry: RetryPolicy::default(), } } @@ -43,6 +47,7 @@ impl GmailApiClient { http: build_http_client(), access_token: access_token.to_string(), base_url: base_url.to_string(), + retry: RetryPolicy::fast(), } } @@ -56,7 +61,7 @@ impl GmailApiClient { .http .get(format!("{}/gmail/v1/users/me/profile", self.base_url)) .bearer_auth(&self.access_token) - .send() + .send_retrying(&self.retry) .await? .json() .await?; @@ -94,7 +99,7 @@ impl GmailApiClient { .get(format!("{}/gmail/v1/users/me/messages", self.base_url)) .bearer_auth(&self.access_token) .query(¶ms) - .send() + .send_retrying(&self.retry) .await? .error_for_status()?; let resp: MessageListResponse = resp.json().await?; @@ -117,7 +122,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .query(&[("format", "full")]) - .send() + .send_retrying(&self.retry) .await? .error_for_status()?; let resp: GmailMessage = resp.json().await?; @@ -148,7 +153,7 @@ impl GmailApiClient { .get(format!("{}/gmail/v1/users/me/history", self.base_url)) .bearer_auth(&self.access_token) .query(¶ms) - .send() + .send_retrying(&self.retry) .await?; // Gmail returns 404 once the startHistoryId is too old (history is // only kept for a limited window). Surface that distinctly so the @@ -210,7 +215,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .json(&body) - .send() + .send_retrying(&self.retry) .await? .json() .await?; @@ -226,7 +231,7 @@ impl GmailApiClient { .post(format!("{}/gmail/v1/users/me/messages/send", self.base_url)) .bearer_auth(&self.access_token) .json(&body) - .send() + .send_retrying(&self.retry) .await? .json() .await?; @@ -244,7 +249,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .query(&[("format", "full")]) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -267,7 +272,7 @@ impl GmailApiClient { self.base_url )) .bearer_auth(&self.access_token) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -282,7 +287,7 @@ impl GmailApiClient { .http .get(format!("{}/gmail/v1/users/me/labels", self.base_url)) .bearer_auth(&self.access_token) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -316,7 +321,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .json(&body) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -349,7 +354,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .json(&body) - .send() + .send_retrying(&self.retry) .await? .error_for_status()?; debug!("gmail: batch_modify ok"); @@ -363,7 +368,7 @@ impl GmailApiClient { .get(format!("{}/gmail/v1/users/me/drafts", self.base_url)) .bearer_auth(&self.access_token) .query(&[("maxResults", max_results.to_string())]) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -383,7 +388,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .query(&[("format", "full")]) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -408,7 +413,7 @@ impl GmailApiClient { .post(format!("{}/gmail/v1/users/me/drafts", self.base_url)) .bearer_auth(&self.access_token) .json(&body) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -430,7 +435,7 @@ impl GmailApiClient { )) .bearer_auth(&self.access_token) .json(&body) - .send() + .send_retrying(&self.retry) .await? .error_for_status()? .json() @@ -447,7 +452,7 @@ impl GmailApiClient { self.base_url )) .bearer_auth(&self.access_token) - .send() + .send_retrying(&self.retry) .await? .error_for_status()?; debug!(draft_id, "gmail: delete_draft ok"); @@ -464,7 +469,7 @@ impl GmailApiClient { self.base_url )) .bearer_auth(&self.access_token) - .send() + .send_retrying(&self.retry) .await?; let resp: SendAsListResponse = Self::json_or_scope_error(resp).await?; let count = resp.send_as.as_ref().map(|s| s.len()).unwrap_or(0); @@ -483,7 +488,7 @@ impl GmailApiClient { self.base_url )) .bearer_auth(&self.access_token) - .send() + .send_retrying(&self.retry) .await?; let resp: SendAsAlias = Self::json_or_scope_error(resp).await?; debug!(send_as_email, "gmail: get_send_as ok"); diff --git a/crates/void-gmail/src/api/mod.rs b/crates/void-gmail/src/api/mod.rs index 6476111..2e9ad48 100644 --- a/crates/void-gmail/src/api/mod.rs +++ b/crates/void-gmail/src/api/mod.rs @@ -1,5 +1,6 @@ mod client; mod message; +mod retry; mod types; #[cfg(test)] @@ -7,4 +8,5 @@ mod tests; pub use client::{build_http_client, GmailApiClient}; pub use message::decode_attachment_data; +pub use retry::RetryPolicy; pub use types::*; diff --git a/crates/void-gmail/src/api/retry.rs b/crates/void-gmail/src/api/retry.rs new file mode 100644 index 0000000..6394692 --- /dev/null +++ b/crates/void-gmail/src/api/retry.rs @@ -0,0 +1,311 @@ +//! Retry policy for transient Gmail API failures. +//! +//! Gmail enforces a per-user quota (`Total Query Cost`, units per minute). It is +//! shared by every process authenticated as that user, so a burst from one client +//! can push another over the limit. Google's answer to that is documented: back off +//! and retry, honouring `Retry-After` when it is present. +//! +//! Without this, a 429 surfaces as a fatal `GmailError::Http` and the caller loses +//! a read that would have succeeded a second later. + +use std::time::Duration; + +use tracing::warn; + +/// Bounded exponential backoff with jitter. +#[derive(Debug, Clone, Copy)] +pub struct RetryPolicy { + /// Total attempts, including the first one. `1` disables retrying. + pub max_attempts: u32, + /// Delay before the second attempt; doubles after each failure. + pub base_delay: Duration, + /// Upper bound on any single delay, before jitter. + pub max_delay: Duration, +} + +impl Default for RetryPolicy { + fn default() -> Self { + Self { + max_attempts: 4, + base_delay: Duration::from_millis(500), + max_delay: Duration::from_secs(16), + } + } +} + +impl RetryPolicy { + /// Near-zero delays, for tests that assert retry behaviour without sleeping. + #[cfg(test)] + pub fn fast() -> Self { + Self { + max_attempts: 4, + base_delay: Duration::from_millis(1), + max_delay: Duration::from_millis(4), + } + } + + /// Delay before attempt number `attempt` (1-based: `1` is the first retry). + /// + /// Full jitter, as recommended for shared quotas: without it, several clients + /// that hit the same 429 would wake up together and collide again. + fn delay_for(&self, attempt: u32) -> Duration { + let exp = self + .base_delay + .saturating_mul(2u32.saturating_pow(attempt.saturating_sub(1))); + let capped = exp.min(self.max_delay); + jitter(capped) + } +} + +/// Full jitter in `[capped / 2, capped]`. +/// +/// Uses the clock rather than a `rand` dependency: the quality needed here is +/// "two processes do not wake up in lockstep", not cryptographic randomness. +fn jitter(capped: Duration) -> Duration { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos() as u64) + .unwrap_or(0); + let half = capped / 2; + let spread = capped.saturating_sub(half); + if spread.is_zero() { + return capped; + } + half + Duration::from_nanos(nanos % (spread.as_nanos() as u64).max(1)) +} + +/// Whether the status alone is enough to retry. +/// +/// 429 and 5xx are transient by definition. 401 and 404 are real answers and must +/// keep failing fast. 403 is decided by the body instead: see +/// [`needs_body_to_decide`]. +fn is_retryable(status: reqwest::StatusCode) -> bool { + status == reqwest::StatusCode::TOO_MANY_REQUESTS || status.is_server_error() +} + +/// Whether the status is ambiguous, so the body has to be read to decide. +/// +/// Gmail does not answer 429 for the per-user quota. It answers **403 with +/// `reason: rateLimitExceeded`**, the same status it uses for a missing scope or a +/// denied permission. One is transient, the others never clear. The discriminator +/// is Google's `reason` field, parsed by +/// [`crate::error::is_retryable_quota_body`]. +fn needs_body_to_decide(status: reqwest::StatusCode) -> bool { + status == reqwest::StatusCode::FORBIDDEN +} + +/// Read the body once, and hand back a response the caller can still consume. +/// +/// Deciding on a 403 costs the response: `bytes()` takes ownership. So the parts +/// are put back together, URL and headers included, and the rebuilt response +/// behaves like the original for `error_for_status`, `text` and `json`. Without +/// this, a non-retryable 403 would reach the caller with an empty body and its +/// error message would lose the Google reason that explains it. +async fn buffer_body( + resp: reqwest::Response, +) -> Result<(reqwest::Response, String), reqwest::Error> { + use reqwest::ResponseBuilderExt; + + let url = resp.url().clone(); + let status = resp.status(); + let headers = resp.headers().clone(); + let bytes = resp.bytes().await?; + let text = String::from_utf8_lossy(&bytes).into_owned(); + + let mut builder = http::Response::builder().status(status).url(url); + if let Some(slot) = builder.headers_mut() { + *slot = headers; + } + // Infallible: the status and headers come from a response that already + // parsed, so there is nothing left for the builder to reject. + let rebuilt = builder + .body(bytes) + .expect("rebuilding a response from its own parts"); + Ok((reqwest::Response::from(rebuilt), text)) +} + +/// Whether a transport error is worth retrying (timeouts and connection failures). +fn is_retryable_transport(err: &reqwest::Error) -> bool { + err.is_timeout() || err.is_connect() +} + +/// `Retry-After`, when the server sent one. Seconds, or an HTTP date. +fn retry_after(resp: &reqwest::Response) -> Option { + let raw = resp + .headers() + .get(reqwest::header::RETRY_AFTER)? + .to_str() + .ok()?; + if let Ok(secs) = raw.trim().parse::() { + return Some(Duration::from_secs(secs)); + } + let when = chrono::DateTime::parse_from_rfc2822(raw.trim()).ok()?; + let delta = when.timestamp() - chrono::Utc::now().timestamp(); + (delta > 0).then(|| Duration::from_secs(delta as u64)) +} + +/// Send a request, retrying transient failures per `policy`. +/// +/// Returns the last response when attempts run out, so the caller's existing +/// `.error_for_status()` still decides the final error. That keeps the error type +/// of every call site unchanged. +/// +/// A request whose body cannot be cloned (streaming) is sent exactly once. +pub async fn send_with_retry( + req: reqwest::RequestBuilder, + policy: &RetryPolicy, +) -> Result { + let mut attempt = 1u32; + loop { + let clone = req.try_clone(); + let is_last = attempt >= policy.max_attempts || clone.is_none(); + + let this = match clone { + Some(c) if !is_last => c, + // Last attempt, or an unclonable body: consume the original. + _ => return req.send().await, + }; + + match this.send().await { + Ok(resp) if is_retryable(resp.status()) => { + let status = resp.status(); + let wait = retry_after(&resp).unwrap_or_else(|| policy.delay_for(attempt)); + warn!( + %status, + attempt, + max_attempts = policy.max_attempts, + wait_ms = wait.as_millis() as u64, + "gmail: transient API failure, backing off" + ); + tokio::time::sleep(wait).await; + } + Ok(resp) if needs_body_to_decide(resp.status()) => { + let status = resp.status(); + let wait = retry_after(&resp).unwrap_or_else(|| policy.delay_for(attempt)); + let (resp, body) = buffer_body(resp).await?; + if !crate::error::is_retryable_quota_body(&body) { + // A real 403: missing scope, denied permission, domain policy. + // Retrying burns quota and hides the problem. + return Ok(resp); + } + warn!( + %status, + attempt, + max_attempts = policy.max_attempts, + wait_ms = wait.as_millis() as u64, + "gmail: quota exceeded (403 rateLimitExceeded), backing off" + ); + tokio::time::sleep(wait).await; + } + Ok(resp) => return Ok(resp), + Err(e) if is_retryable_transport(&e) => { + let wait = policy.delay_for(attempt); + warn!( + attempt, + max_attempts = policy.max_attempts, + wait_ms = wait.as_millis() as u64, + "gmail: transport error, backing off: {e}" + ); + tokio::time::sleep(wait).await; + } + Err(e) => return Err(e), + } + attempt += 1; + } +} + +/// Lets a call site opt into retrying by replacing `.send()` with +/// `.send_retrying(&self.retry)`, keeping the rest of the chain untouched. +pub trait SendRetrying { + /// Send with the given retry policy. See [`send_with_retry`]. + fn send_retrying( + self, + policy: &RetryPolicy, + ) -> impl std::future::Future>; +} + +impl SendRetrying for reqwest::RequestBuilder { + async fn send_retrying( + self, + policy: &RetryPolicy, + ) -> Result { + send_with_retry(self, policy).await + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn retryable_covers_quota_and_backend_errors() { + assert!(is_retryable(reqwest::StatusCode::TOO_MANY_REQUESTS)); + assert!(is_retryable(reqwest::StatusCode::INTERNAL_SERVER_ERROR)); + assert!(is_retryable(reqwest::StatusCode::SERVICE_UNAVAILABLE)); + } + + #[test] + fn retryable_excludes_real_answers() { + // A missing thread must keep failing fast: retrying it burns quota for an + // answer that will not change. + assert!(!is_retryable(reqwest::StatusCode::UNAUTHORIZED)); + assert!(!is_retryable(reqwest::StatusCode::NOT_FOUND)); + assert!(!is_retryable(reqwest::StatusCode::OK)); + } + + #[test] + fn forbidden_is_decided_by_the_body_not_the_status() { + // 403 is the status Gmail actually sends for the per-user quota, and also + // the one it sends for a missing scope. The status alone decides nothing. + assert!(!is_retryable(reqwest::StatusCode::FORBIDDEN)); + assert!(needs_body_to_decide(reqwest::StatusCode::FORBIDDEN)); + assert!(!needs_body_to_decide(reqwest::StatusCode::NOT_FOUND)); + assert!(!needs_body_to_decide( + reqwest::StatusCode::TOO_MANY_REQUESTS + )); + assert!(!needs_body_to_decide(reqwest::StatusCode::OK)); + } + + #[tokio::test] + async fn buffer_body_preserves_status_headers_and_body() { + use reqwest::ResponseBuilderExt; + + let original = reqwest::Response::from( + http::Response::builder() + .status(403) + .header("X-Marker", "kept") + .url(url::Url::parse("https://example.test/threads/t1").unwrap()) + .body("quota body") + .unwrap(), + ); + + let (rebuilt, text) = buffer_body(original).await.unwrap(); + assert_eq!(text, "quota body"); + assert_eq!(rebuilt.status(), reqwest::StatusCode::FORBIDDEN); + assert_eq!( + rebuilt + .headers() + .get("X-Marker") + .map(|v| v.to_str().unwrap()), + Some("kept") + ); + assert_eq!(rebuilt.url().as_str(), "https://example.test/threads/t1"); + // The caller still gets the body: reading it to decide must not consume it. + assert_eq!(rebuilt.text().await.unwrap(), "quota body"); + } + + #[test] + fn delay_grows_and_stays_capped() { + let p = RetryPolicy { + max_attempts: 6, + base_delay: Duration::from_millis(100), + max_delay: Duration::from_millis(800), + }; + // Jitter puts each delay in [capped/2, capped]. + assert!(p.delay_for(1) >= Duration::from_millis(50)); + assert!(p.delay_for(1) <= Duration::from_millis(100)); + assert!(p.delay_for(3) <= Duration::from_millis(400)); + // Far-out attempts stay bounded by max_delay. + assert!(p.delay_for(20) <= Duration::from_millis(800)); + } +} diff --git a/crates/void-gmail/src/api/tests.rs b/crates/void-gmail/src/api/tests.rs index 736fa0d..1521bca 100644 --- a/crates/void-gmail/src/api/tests.rs +++ b/crates/void-gmail/src/api/tests.rs @@ -200,9 +200,12 @@ async fn list_labels_401_preserves_status() { } } -/// `get_thread` preserves status via `.error_for_status()`. +/// `get_thread` still surfaces a 5xx once retries are exhausted. +/// +/// The client now retries 5xx, so the mock answers 500 every time and the error +/// must survive the last attempt unchanged. #[tokio::test] -async fn get_thread_500_preserves_status() { +async fn get_thread_500_preserves_status_after_retries() { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/gmail/v1/users/me/threads/t1")) @@ -220,9 +223,13 @@ async fn get_thread_500_preserves_status() { } } -/// `create_draft` preserves status (e.g. 429 rate-limit) via `.error_for_status()`. +/// `create_draft` still surfaces a 429 once retries are exhausted. +/// +/// Changed deliberately: this test used to assert that a 429 failed on the first +/// response. The client now retries transient failures, so what must hold is that +/// the status is preserved when every attempt fails, not that only one is made. #[tokio::test] -async fn create_draft_429_preserves_status() { +async fn create_draft_429_preserves_status_after_retries() { let server = MockServer::start().await; Mock::given(method("POST")) .and(path("/gmail/v1/users/me/drafts")) @@ -375,3 +382,229 @@ async fn resolve_signature_other_forbidden_is_not_insufficient_scope() { other => panic!("expected Api error, got {other:?}"), } } + +// -- Retry on transient failures -- + +/// A 429 followed by a 200 must resolve to the 200: the caller never sees the +/// rate limit. This is the production case, where a sibling process transiently +/// consumed the shared per-user quota. +#[tokio::test] +async fn get_message_retries_429_then_succeeds() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/m1")) + .respond_with(ResponseTemplate::new(429).set_body_string("rate limited")) + .up_to_n_times(1) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/m1")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": "m1", + "threadId": "t1", + "internalDate": "1741700000000" + }))) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let msg = api.get_message("m1").await.expect("retry should recover"); + assert_eq!(msg.id.as_deref(), Some("m1")); +} + +/// `Retry-After` is honoured rather than ignored in favour of the backoff curve. +#[tokio::test] +async fn get_message_honours_retry_after_header() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/m2")) + .respond_with( + ResponseTemplate::new(429) + .insert_header("Retry-After", "0") + .set_body_string("rate limited"), + ) + .up_to_n_times(1) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/m2")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": "m2", + "threadId": "t2", + "internalDate": "1741700000000" + }))) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let msg = api.get_message("m2").await.expect("retry should recover"); + assert_eq!(msg.id.as_deref(), Some("m2")); +} + +/// A 5xx that clears on the second attempt must not reach the caller. +#[tokio::test] +async fn get_thread_retries_500_then_succeeds() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/threads/t9")) + .respond_with(ResponseTemplate::new(503).set_body_string("backend error")) + .up_to_n_times(1) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/threads/t9")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": "t9", + "messages": [] + }))) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let thread = api.get_thread("t9").await.expect("retry should recover"); + assert_eq!(thread.id.as_deref(), Some("t9")); +} + +/// A 404 must NOT be retried: it is a real answer, and retrying it would burn +/// quota waiting for a result that cannot change. `expect(1)` is the assertion. +#[tokio::test] +async fn get_message_does_not_retry_404() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/gone")) + .respond_with(ResponseTemplate::new(404).set_body_string("not found")) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let err = api.get_message("gone").await.expect_err("expected error"); + match err { + GmailError::Http(e) => assert_eq!(e.status(), Some(reqwest::StatusCode::NOT_FOUND)), + other => panic!("expected Http error, got {other:?}"), + } + // Dropping the server verifies the `expect(1)`: a retry would make it 2. +} + +// -- Retry on 403, decided by Google's `reason` -- + +/// Gmail's real per-user quota answer. Measured on a shared account over a day: +/// 16 of these, and zero 429. Retrying only 429 therefore missed every one. +const QUOTA_403: &str = r#"{"error":{"code":403,"message":"User-rate limit exceeded. Retry after 2026-09-11T20:00:00.000Z","errors":[{"message":"User-rate limit exceeded.","domain":"usageLimits","reason":"rateLimitExceeded"}],"status":"PERMISSION_DENIED"}}"#; + +/// A scope error carries the same 403 and must keep failing fast. +const SCOPE_403: &str = r#"{"error":{"code":403,"message":"Request had insufficient authentication scopes.","status":"PERMISSION_DENIED","details":[{"@type":"type.googleapis.com/google.rpc.ErrorInfo","reason":"ACCESS_TOKEN_SCOPE_INSUFFICIENT"}]}}"#; + +/// 403 + `rateLimitExceeded`, then 200: the caller never sees the quota error. +/// This is the failure that motivated the change. +#[tokio::test] +async fn get_message_retries_403_rate_limit_then_succeeds() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/m3")) + .respond_with(ResponseTemplate::new(403).set_body_string(QUOTA_403)) + .up_to_n_times(1) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/m3")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": "m3", + "threadId": "t3", + "internalDate": "1741700000000" + }))) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let msg = api.get_message("m3").await.expect("retry should recover"); + assert_eq!(msg.id.as_deref(), Some("m3")); +} + +/// 403 + `ACCESS_TOKEN_SCOPE_INSUFFICIENT` must NOT be retried. `expect(1)` is the +/// assertion: retrying a scope error burns quota and hides a real auth problem. +#[tokio::test] +async fn get_message_does_not_retry_403_scope_error() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/messages/scoped")) + .respond_with(ResponseTemplate::new(403).set_body_string(SCOPE_403)) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let err = api.get_message("scoped").await.expect_err("expected error"); + match err { + GmailError::Http(e) => assert_eq!(e.status(), Some(reqwest::StatusCode::FORBIDDEN)), + other => panic!("expected Http error, got {other:?}"), + } + // Dropping the server verifies the `expect(1)`: a retry would make it 2. +} + +/// A 403 that is not a quota error keeps its body, not just its status. +/// +/// The retry path reads the body to decide, which consumes the response. If it +/// were not rebuilt, this 403 would reach the caller empty and the Google reason +/// that explains it would be lost. +#[tokio::test] +async fn resolve_signature_403_scope_error_keeps_its_body() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/settings/sendAs")) + .respond_with(ResponseTemplate::new(403).set_body_string(SCOPE_403)) + .expect(1) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let err = api.resolve_signature(None).await.expect_err("expected 403"); + // Reaching InsufficientScope proves the body survived: the client maps it by + // parsing the reason out of the body, not from the status. + assert!( + matches!(err, GmailError::InsufficientScope), + "expected InsufficientScope, got {err:?}" + ); +} + +/// 403 + `rateLimitExceeded` on every attempt: the caller still gets 403, and the +/// body still carries Google's reason. +/// +/// Two assertions on one mock: the typed client path preserves the status, and the +/// retry helper itself hands back a response whose body was not eaten by the read +/// that decided to retry. +#[tokio::test] +async fn get_thread_403_rate_limit_preserves_status_and_body_after_retries() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/gmail/v1/users/me/threads/t403")) + .respond_with(ResponseTemplate::new(403).set_body_string(QUOTA_403)) + .mount(&server) + .await; + + let api = GmailApiClient::with_base_url("test-token", &server.uri()); + let err = api.get_thread("t403").await.expect_err("expected error"); + match err { + GmailError::Http(e) => assert_eq!(e.status(), Some(reqwest::StatusCode::FORBIDDEN)), + other => panic!("expected Http error, got {other:?}"), + } + + // `error_for_status` keeps the status but drops the payload, so the body is + // asserted one level down, on what the retry loop actually returned. + let resp = retry::send_with_retry( + reqwest::Client::new().get(format!("{}/gmail/v1/users/me/threads/t403", server.uri())), + &RetryPolicy::fast(), + ) + .await + .expect("retries exhausted, not a transport error"); + assert_eq!(resp.status(), reqwest::StatusCode::FORBIDDEN); + assert!(resp.text().await.unwrap().contains("rateLimitExceeded")); +} diff --git a/crates/void-gmail/src/error.rs b/crates/void-gmail/src/error.rs index 03f0a93..21bafd7 100644 --- a/crates/void-gmail/src/error.rs +++ b/crates/void-gmail/src/error.rs @@ -27,18 +27,85 @@ pub enum GmailError { Other(String), } +/// The `reason` values carried by a Google API error body, lowercased. +/// +/// Google uses two shapes for the same field: the classic +/// `error.errors[].reason` and the newer `error.details[].reason`. Both are read +/// here so every caller matches against one list instead of learning the shapes +/// again. A body that is not JSON yields nothing, and callers fall back to +/// matching the reason token in the raw text. +fn google_error_reasons(body: &str) -> Vec { + let Ok(parsed) = serde_json::from_str::(body) else { + return Vec::new(); + }; + let Some(error) = parsed.get("error") else { + return Vec::new(); + }; + ["errors", "details"] + .iter() + .filter_map(|key| error.get(key)?.as_array()) + .flatten() + .filter_map(|item| item.get("reason")?.as_str()) + .map(|reason| reason.to_ascii_lowercase()) + .collect() +} + +/// Google reasons that mean "the request was fine, come back later". +/// +/// `rateLimitExceeded` and `userRateLimitExceeded` are the per-user quota, +/// `quotaExceeded` the project quota, `backendError` a transient Gmail fault. +const RETRYABLE_REASONS: [&str; 4] = [ + "ratelimitexceeded", + "userratelimitexceeded", + "quotaexceeded", + "backenderror", +]; + /// Whether a Gmail API error body indicates missing OAuth scopes (vs other 403s). /// /// Matches Google's `ACCESS_TOKEN_SCOPE_INSUFFICIENT` reason and the common /// "insufficient authentication scopes" message. Avoids broad phrases like /// "insufficient permissions", which appear on unrelated 403s. pub fn is_insufficient_scope_body(body: &str) -> bool { + let reasons = google_error_reasons(body); + if reasons + .iter() + .any(|r| r == "access_token_scope_insufficient" || r == "insufficientpermissions") + { + return true; + } let lower = body.to_ascii_lowercase(); lower.contains("access_token_scope_insufficient") || lower.contains("insufficientpermissions") || lower.contains("insufficient authentication scopes") } +/// Whether a Gmail API error body says the quota was hit, so the call is worth +/// retrying after a backoff. +/// +/// Needed because Gmail does not answer 429 for the per-user quota: it answers +/// **403 with `reason: rateLimitExceeded`**. The status alone therefore cannot +/// decide, and every other 403 (missing scope, denied permission, domain policy) +/// is a real answer that must keep failing fast. Retrying those burns quota and +/// hides an auth problem. +pub fn is_retryable_quota_body(body: &str) -> bool { + // A scope error wins: it is a 403 that will never clear on its own, and some + // bodies mention both a permission reason and quota-looking prose. + if is_insufficient_scope_body(body) { + return false; + } + if google_error_reasons(body) + .iter() + .any(|r| RETRYABLE_REASONS.contains(&r.as_str())) + { + return true; + } + // Non-JSON or an unparsed shape: match the reason token itself, never loose + // prose like "quota", which shows up in unrelated messages. + let lower = body.to_ascii_lowercase(); + RETRYABLE_REASONS.iter().any(|r| lower.contains(r)) +} + #[cfg(test)] mod tests { use super::*; @@ -63,4 +130,57 @@ mod tests { assert!(!is_insufficient_scope_body("insufficient permissions")); assert!(!is_insufficient_scope_body("")); } + + #[test] + fn google_error_reasons_reads_both_shapes() { + assert_eq!( + google_error_reasons(r#"{"error":{"errors":[{"reason":"rateLimitExceeded"}]}}"#), + vec!["ratelimitexceeded"] + ); + assert_eq!( + google_error_reasons(r#"{"error":{"details":[{"reason":"backendError"}]}}"#), + vec!["backenderror"] + ); + assert!(google_error_reasons("not json at all").is_empty()); + assert!(google_error_reasons(r#"{"something":"else"}"#).is_empty()); + } + + #[test] + fn retryable_quota_body_detects_the_403_gmail_actually_sends() { + // The real payload, trimmed: this is what the per-user quota looks like. + assert!(is_retryable_quota_body( + r#"{"error":{"code":403,"message":"User-rate limit exceeded. Retry after 2026-09-11T20:00:00.000Z","errors":[{"message":"User-rate limit exceeded.","domain":"usageLimits","reason":"rateLimitExceeded"}],"status":"PERMISSION_DENIED"}}"# + )); + assert!(is_retryable_quota_body( + r#"{"error":{"errors":[{"reason":"userRateLimitExceeded"}]}}"# + )); + assert!(is_retryable_quota_body( + r#"{"error":{"errors":[{"reason":"quotaExceeded"}]}}"# + )); + assert!(is_retryable_quota_body( + r#"{"error":{"details":[{"reason":"backendError"}]}}"# + )); + } + + #[test] + fn retryable_quota_body_rejects_real_answers() { + // A scope error must keep failing fast: retrying hides the auth problem. + assert!(!is_retryable_quota_body( + r#"{"error":{"message":"Request had insufficient authentication scopes.","status":"PERMISSION_DENIED","details":[{"reason":"ACCESS_TOKEN_SCOPE_INSUFFICIENT"}]}}"# + )); + assert!(!is_retryable_quota_body( + r#"{"error":{"errors":[{"reason":"insufficientPermissions"}]}}"# + )); + assert!(!is_retryable_quota_body( + r#"{"error":{"errors":[{"reason":"forbidden"}]}}"# + )); + assert!(!is_retryable_quota_body( + "Admin has disabled this API for the domain." + )); + // Prose about quotas is not a reason. Only the reason token counts. + assert!(!is_retryable_quota_body( + "You have exceeded your daily quota of patience." + )); + assert!(!is_retryable_quota_body("")); + } }