Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

Commit

Permalink
fix(object-store): Consider more GCS errors transient (matter-labs#2246)
Browse files Browse the repository at this point in the history
## What ❔

Considers `reqwest::Error`s with "request" kind transient for the object
store logic.

## Why ❔

Similar to some other kinds of `reqwest::Error`s, not all "request"
errors are truly transient, but considering them as such is a safer
option.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
  • Loading branch information
slowli authored and gabrieldemian committed Jun 21, 2024
1 parent 7ce721a commit 4e86dfc
Showing 1 changed file with 13 additions and 7 deletions.
20 changes: 13 additions & 7 deletions core/lib/object_store/src/gcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl From<AuthError> for ObjectStoreError {
fn from(err: AuthError) -> Self {
let is_transient = matches!(
&err,
AuthError::HttpError(err) if err.is_timeout() || err.is_connect() || has_transient_io_source(err) || upstream_unavailable(err)
AuthError::HttpError(err) if is_transient_http_error(err)
);
Self::Initialization {
source: err.into(),
Expand All @@ -96,6 +96,17 @@ impl From<AuthError> for ObjectStoreError {
}
}

fn is_transient_http_error(err: &reqwest::Error) -> bool {
err.is_timeout()
|| err.is_connect()
// Not all request errors are logically transient, but a significant part of them are (e.g.,
// `hyper` protocol-level errors), and it's safer to consider an error transient.
|| err.is_request()
|| has_transient_io_source(err)
|| err.status() == Some(StatusCode::BAD_GATEWAY)
|| err.status() == Some(StatusCode::SERVICE_UNAVAILABLE)
}

fn has_transient_io_source(mut err: &(dyn StdError + 'static)) -> bool {
loop {
if err.is::<io::Error>() {
Expand All @@ -111,11 +122,6 @@ fn has_transient_io_source(mut err: &(dyn StdError + 'static)) -> bool {
}
}

fn upstream_unavailable(err: &reqwest::Error) -> bool {
err.status() == Some(StatusCode::BAD_GATEWAY)
|| err.status() == Some(StatusCode::SERVICE_UNAVAILABLE)
}

impl From<HttpError> for ObjectStoreError {
fn from(err: HttpError) -> Self {
let is_not_found = match &err {
Expand All @@ -131,7 +137,7 @@ impl From<HttpError> for ObjectStoreError {
} else {
let is_transient = matches!(
&err,
HttpError::HttpClient(err) if err.is_timeout() || err.is_connect() || has_transient_io_source(err) || upstream_unavailable(err)
HttpError::HttpClient(err) if is_transient_http_error(err)
);
ObjectStoreError::Other {
is_transient,
Expand Down

0 comments on commit 4e86dfc

Please sign in to comment.