mirror of
https://github.com/narrrl/proton-sdk-rs.git
synced 2026-11-03 16:41:26 +01:00
feat(drive,sdk): port upstream C# SDK up to 6cbf2f44
Port 6cbf2f44 (stop retrying permanent API errors on block transfers) and 20822176 (small file upload interop); 6ce47922 is a no-op here. - Add ProtonApiError::is_retriable / ProtonError::is_retriable mirroring C# RetryPolicy.IsRetriable: 4xx is permanent except 404, 408 and 429, with envelope codes able to override the HTTP status. - Gate upload_one_block's retry loop on it so a permanent rejection of a revision fails immediately instead of being replayed three times. - Extract BlockTargets from RevisionReader so the whole-file download path, the public link download and the reader all re-mint expired block URLs through one generation-deduplicated refresh. - Fall back from the small upload to the block path on AlreadyExists only; the small upload is not idempotent, so everything else keeps propagating. - Bound a server-supplied Retry-After by RetryPolicy::max_retry_after (default 120 s) so a long rate-limit answer cannot look like a hang.
This commit is contained in:
1 parent
4f129b778a
commit
1ca28c5848
7 files changed
+420
-113
No files matched your search
@@ -28,16 +28,25 @@ pub const DEFAULT_MAX_RETRIES: u32 = 3;
|
||||
/// Default base delay for exponential backoff between retries.
|
||||
pub const DEFAULT_RETRY_BASE_DELAY: Duration = Duration::from_millis(500);
|
||||
|
||||
/// Default upper bound for a single backoff sleep. A server-supplied
|
||||
/// `Retry-After` is honoured even if it exceeds this.
|
||||
/// Default upper bound for a single computed backoff sleep. A server-supplied
|
||||
/// `Retry-After` overrides it, up to [`DEFAULT_MAX_RETRY_AFTER`].
|
||||
pub const DEFAULT_RETRY_MAX_DELAY: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Default upper bound for a server-supplied `Retry-After` wait.
|
||||
///
|
||||
/// The header is advice, and a caller is blocked on the sleep for its whole
|
||||
/// duration: an unbounded one is indistinguishable, from outside, from a hung
|
||||
/// process. Waiting less than asked risks another rate-limit answer, which the
|
||||
/// retry budget absorbs; waiting an hour risks a daemon that looks dead.
|
||||
pub const DEFAULT_MAX_RETRY_AFTER: Duration = Duration::from_secs(120);
|
||||
|
||||
/// Controls automatic retries on retryable responses (HTTP 408/429/502/503/504)
|
||||
/// and transient transport errors (timeout, connect).
|
||||
///
|
||||
/// Mirrors the Polly-style retry pipeline in the C# SDK: a server-supplied
|
||||
/// `Retry-After` header wins; otherwise the delay is exponential backoff
|
||||
/// (`base_delay * 2^attempt`, capped at `max_delay`) with full jitter.
|
||||
/// `Retry-After` header wins (clamped to `max_retry_after`); otherwise the delay
|
||||
/// is exponential backoff (`base_delay * 2^attempt`, capped at `max_delay`) with
|
||||
/// full jitter.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct RetryPolicy {
|
||||
/// Maximum number of retries after the initial attempt. `0` disables retry.
|
||||
@@ -46,6 +55,9 @@ pub struct RetryPolicy {
|
||||
pub base_delay: Duration,
|
||||
/// Cap on a single computed backoff sleep.
|
||||
pub max_delay: Duration,
|
||||
/// Cap on a server-supplied `Retry-After` wait. A longer header value is
|
||||
/// clamped to this.
|
||||
pub max_retry_after: Duration,
|
||||
}
|
||||
|
||||
impl Default for RetryPolicy {
|
||||
@@ -54,6 +66,7 @@ impl Default for RetryPolicy {
|
||||
max_retries: DEFAULT_MAX_RETRIES,
|
||||
base_delay: DEFAULT_RETRY_BASE_DELAY,
|
||||
max_delay: DEFAULT_RETRY_MAX_DELAY,
|
||||
max_retry_after: DEFAULT_MAX_RETRY_AFTER,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,6 +32,24 @@ impl ProtonError {
|
||||
pub fn invalid_operation(message: impl Into<String>) -> Self {
|
||||
Self::InvalidOperation(message.into())
|
||||
}
|
||||
|
||||
/// Whether replaying the request that produced this error could plausibly
|
||||
/// succeed (C# `RetryPolicy.IsRetriable`).
|
||||
///
|
||||
/// Used by the retry loops that sit *above* the HTTP client — block upload
|
||||
/// and block download — which retry a whole prepared transfer rather than a
|
||||
/// single request, and so cannot rely on [`crate::http`]'s own policy. A
|
||||
/// permanent rejection of the revision must fail the transfer at once
|
||||
/// instead of being replayed until the attempt budget runs out.
|
||||
pub fn is_retriable(&self) -> bool {
|
||||
match self {
|
||||
Self::Api(error) => error.is_retriable(),
|
||||
// A serialization or crypto failure is deterministic; an invariant
|
||||
// violation is a bug. Only a transport failure is worth replaying.
|
||||
Self::Transport(_) => true,
|
||||
Self::Serialization(_) | Self::Crypto(_) | Self::InvalidOperation(_) => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// An error reported by the Proton API in its response envelope.
|
||||
@@ -50,6 +68,23 @@ pub struct ProtonApiError {
|
||||
}
|
||||
|
||||
impl ProtonApiError {
|
||||
/// Whether replaying the request could plausibly succeed.
|
||||
///
|
||||
/// A 4xx cannot succeed on a replay, except 408 and 429, and 404 — which is
|
||||
/// retriable only because both block-transfer callers re-request the
|
||||
/// transfer target first (C# `RetryPolicy.StatusCodeIsRetriable`). The
|
||||
/// envelope code is consulted as well as the HTTP status, because a rate
|
||||
/// limit can arrive as `Code: 429` under a 2xx-shaped envelope.
|
||||
pub fn is_retriable(&self) -> bool {
|
||||
if matches!(
|
||||
self.code,
|
||||
ResponseCode::TooManyRequests | ResponseCode::DoesNotExist
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
!(400..500).contains(&self.http_status) || matches!(self.http_status, 404 | 408 | 429)
|
||||
}
|
||||
|
||||
pub fn is_unauthorized(&self) -> bool {
|
||||
self.http_status == 401
|
||||
}
|
||||
@@ -81,3 +116,55 @@ impl ProtonApiError {
|
||||
serde_json::from_value(self.details.clone()?).ok()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn api(http_status: u16, code: ResponseCode) -> ProtonError {
|
||||
ProtonError::Api(ProtonApiError {
|
||||
code,
|
||||
http_status,
|
||||
message: String::new(),
|
||||
details: None,
|
||||
})
|
||||
}
|
||||
|
||||
/// The retriable set a block transfer may replay: transient statuses, plus
|
||||
/// 404, which means the transfer target expired and the caller re-mints it.
|
||||
#[test]
|
||||
fn transient_statuses_and_404_are_retriable() {
|
||||
for status in [404, 408, 429, 500, 502, 503, 504] {
|
||||
assert!(
|
||||
api(status, ResponseCode::Unknown).is_retriable(),
|
||||
"http {status} should be retriable"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Everything else in the 4xx family is a permanent rejection: replaying it
|
||||
/// only spends the attempt budget.
|
||||
#[test]
|
||||
fn other_client_errors_are_not_retriable() {
|
||||
for status in [400, 401, 403, 409, 422] {
|
||||
assert!(
|
||||
!api(status, ResponseCode::Unknown).is_retriable(),
|
||||
"http {status} should not be retriable"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A rate limit can arrive as an envelope code under a 2xx-shaped response,
|
||||
/// and a missing block as `DoesNotExist`; both still mean "try again".
|
||||
#[test]
|
||||
fn envelope_codes_override_a_permanent_status() {
|
||||
assert!(api(400, ResponseCode::TooManyRequests).is_retriable());
|
||||
assert!(api(422, ResponseCode::DoesNotExist).is_retriable());
|
||||
}
|
||||
|
||||
/// Non-API failures: only a transport error is worth replaying.
|
||||
#[test]
|
||||
fn only_transport_failures_retry_off_the_api_path() {
|
||||
assert!(!ProtonError::invalid_operation("bug").is_retriable());
|
||||
}
|
||||
}
|
||||
@@ -738,8 +738,9 @@ fn api_error(status: StatusCode, bytes: &[u8]) -> ProtonError {
|
||||
/// `multipart` can't be cloned), so every retry resends the full request.
|
||||
///
|
||||
/// Retryable = HTTP 408/429/502/503/504 or a transient transport error
|
||||
/// (timeout / connect). A `Retry-After` header (delta-seconds) is honoured;
|
||||
/// otherwise the delay is exponential backoff with full jitter. Non-retryable
|
||||
/// (timeout / connect). A `Retry-After` header (delta-seconds) is honoured, up
|
||||
/// to `policy.max_retry_after`; otherwise the delay is exponential backoff with
|
||||
/// full jitter. Non-retryable
|
||||
/// responses and errors — including ordinary 4xx and the 401 that drives token
|
||||
/// refresh — pass straight through to the caller untouched.
|
||||
async fn send_retrying<F>(policy: &RetryPolicy, build: F) -> Result<reqwest::Response>
|
||||
@@ -751,7 +752,9 @@ where
|
||||
match build().send().await {
|
||||
Ok(response) => {
|
||||
if attempt < policy.max_retries && is_retryable_status(response.status()) {
|
||||
let delay = retry_after(&response).unwrap_or_else(|| backoff(policy, attempt));
|
||||
let delay = retry_after(&response)
|
||||
.map(|delay| delay.min(policy.max_retry_after))
|
||||
.unwrap_or_else(|| backoff(policy, attempt));
|
||||
tokio::time::sleep(delay).await;
|
||||
attempt += 1;
|
||||
continue;
|
||||
@@ -852,12 +855,28 @@ mod tests {
|
||||
assert_eq!(parse_retry_after_secs(""), None);
|
||||
}
|
||||
|
||||
/// A `Retry-After` longer than the policy allows is clamped: the caller is
|
||||
/// blocked on this sleep, and an unbounded one looks exactly like a hang.
|
||||
#[test]
|
||||
fn retry_after_is_clamped_to_policy_ceiling() {
|
||||
let policy = RetryPolicy::default();
|
||||
let advised = parse_retry_after_secs("3600").expect("delta-seconds parses");
|
||||
assert_eq!(
|
||||
advised.min(policy.max_retry_after),
|
||||
crate::config::DEFAULT_MAX_RETRY_AFTER
|
||||
);
|
||||
// A shorter wait is honoured verbatim.
|
||||
let short = parse_retry_after_secs("5").expect("delta-seconds parses");
|
||||
assert_eq!(short.min(policy.max_retry_after), Duration::from_secs(5));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backoff_grows_then_caps_within_jitter_bounds() {
|
||||
let policy = RetryPolicy {
|
||||
max_retries: 5,
|
||||
base_delay: Duration::from_millis(100),
|
||||
max_delay: Duration::from_millis(1000),
|
||||
..RetryPolicy::default()
|
||||
};
|
||||
// Full jitter: every sample stays within [0, ceiling] where the
|
||||
// ceiling is base*2^attempt capped at max_delay.
|
||||
|
||||
Reference in new issue
Block a user