Skip to content

Commit 9b5024d

Browse files
ref: Replace unmaintained backoff with backon (#2816)
Resolves [BE-578](https://linear.app/getsentry/issue/BE-578/remove-backoff-dependency)
1 parent e700c2a commit 9b5024d

File tree

6 files changed

+81
-62
lines changed

6 files changed

+81
-62
lines changed

Cargo.lock

Lines changed: 19 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ rust-version = "1.86"
1111
[dependencies]
1212
anylog = "0.6.3"
1313
anyhow = { version = "1.0.69", features = ["backtrace"] }
14-
backoff = "0.4.0"
1514
bytecount = "0.6.3"
1615
chrono = { version = "0.4.31", features = ["serde"] }
1716
clap = { version = "4.1.6", default-features = false, features = [
@@ -79,6 +78,7 @@ magic_string = "0.3.4"
7978
chrono-tz = "0.8.4"
8079
secrecy = "0.8.0"
8180
lru = "0.16.0"
81+
backon = { version = "1.5.2", features = ["std", "std-blocking-sleep"] }
8282
brotli = "8.0.2"
8383

8484
[dev-dependencies]

src/api/errors/mod.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,27 @@ mod sentry_error;
44
pub(super) use api_error::{ApiError, ApiErrorKind};
55
pub(super) use sentry_error::SentryError;
66

7+
use crate::api::ApiResponse;
8+
79
#[derive(Clone, Debug, thiserror::Error)]
810
#[error("project was renamed to '{0}'\nPlease use this slug in your .sentryclirc file, sentry.properties file or in the CLI --project parameter")]
911
pub(super) struct ProjectRenamedError(pub(super) String);
1012

1113
/// Shortcut alias for results of this module.
1214
pub(super) type ApiResult<T> = Result<T, ApiError>;
15+
16+
#[derive(Debug, thiserror::Error)]
17+
#[error("request failed with retryable status code {}", .body.status)]
18+
pub(super) struct RetryError {
19+
body: ApiResponse,
20+
}
21+
22+
impl RetryError {
23+
pub fn new(body: ApiResponse) -> Self {
24+
Self { body }
25+
}
26+
27+
pub fn into_body(self) -> ApiResponse {
28+
self.body
29+
}
30+
}

src/api/mod.rs

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ use std::borrow::Cow;
1515
use std::cell::RefCell;
1616
use std::collections::{HashMap, HashSet};
1717
use std::ffi::OsStr;
18-
use std::fmt;
1918
use std::fs::File;
2019
use std::io::{self, Read as _, Write};
2120
use std::path::Path;
2221
use std::rc::Rc;
2322
use std::sync::Arc;
23+
use std::{fmt, thread};
2424

2525
use anyhow::{Context as _, Result};
26-
use backoff::backoff::Backoff as _;
26+
use backon::BlockingRetryable as _;
2727
use brotli::enc::BrotliEncoderParams;
2828
use brotli::CompressorWriter;
2929
#[cfg(target_os = "macos")]
@@ -45,7 +45,7 @@ use symbolic::common::DebugId;
4545
use symbolic::debuginfo::ObjectKind;
4646
use uuid::Uuid;
4747

48-
use crate::api::errors::ProjectRenamedError;
48+
use crate::api::errors::{ProjectRenamedError, RetryError};
4949
use crate::config::{Auth, Config};
5050
use crate::constants::{ARCH, DEFAULT_URL, EXT, PLATFORM, RELEASE_REGISTRY_LATEST_URL, VERSION};
5151
use crate::utils::file_upload::LegacyUploadContext;
@@ -1858,32 +1858,42 @@ impl ApiRequest {
18581858
pub fn send(mut self) -> ApiResult<ApiResponse> {
18591859
let max_retries = Config::current().max_retries();
18601860

1861-
let mut backoff = get_default_backoff();
1862-
let mut retry_number = 0;
1861+
let backoff = get_default_backoff().with_max_times(max_retries as usize);
1862+
let retry_number = RefCell::new(0);
18631863

1864-
loop {
1864+
let send_req = || {
18651865
let mut out = vec![];
1866-
debug!("retry number {retry_number}, max retries: {max_retries}",);
1866+
let mut retry_number = retry_number.borrow_mut();
1867+
1868+
debug!("retry number {retry_number}, max retries: {max_retries}");
1869+
*retry_number += 1;
18671870

18681871
let mut rv = self.send_into(&mut out)?;
1869-
if retry_number >= max_retries || !RETRY_STATUS_CODES.contains(&rv.status) {
1870-
rv.body = Some(out);
1871-
return Ok(rv);
1872-
}
1872+
rv.body = Some(out);
18731873

1874-
// Exponential backoff
1875-
let backoff_timeout = backoff
1876-
.next_backoff()
1877-
.expect("should not return None, as there is no max_elapsed_time");
1874+
if RETRY_STATUS_CODES.contains(&rv.status) {
1875+
anyhow::bail!(RetryError::new(rv));
1876+
}
18781877

1879-
debug!(
1880-
"retry number {retry_number}, retrying again in {} ms",
1881-
backoff_timeout.as_milliseconds()
1882-
);
1883-
std::thread::sleep(backoff_timeout);
1878+
Ok(rv)
1879+
};
18841880

1885-
retry_number += 1;
1886-
}
1881+
send_req
1882+
.retry(backoff)
1883+
.sleep(thread::sleep)
1884+
.when(|e| e.is::<RetryError>())
1885+
.notify(|e, dur| {
1886+
debug!(
1887+
"retry number {} failed due to {e:#}, retrying again in {} ms",
1888+
*retry_number.borrow() - 1,
1889+
dur.as_milliseconds()
1890+
);
1891+
})
1892+
.call()
1893+
.or_else(|err| match err.downcast::<RetryError>() {
1894+
Ok(err) => Ok(err.into_body()),
1895+
Err(err) => Err(ApiError::with_source(ApiErrorKind::RequestFailed, err)),
1896+
})
18871897
}
18881898
}
18891899

src/constants.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@ pub const EXT: &str = ".exe";
3030
pub const EXT: &str = "";
3131

3232
/// Backoff multiplier (1.5 which is 50% increase per backoff).
33-
pub const DEFAULT_MULTIPLIER: f64 = 1.5;
34-
/// Backoff randomization factor (0 means no randomization).
35-
pub const DEFAULT_RANDOMIZATION: f64 = 0.1;
33+
pub const DEFAULT_MULTIPLIER: f32 = 1.5;
3634
/// Initial backoff interval in milliseconds.
3735
pub const DEFAULT_INITIAL_INTERVAL: u64 = 1000;
3836
/// Maximum backoff interval in milliseconds.

src/utils/retry.rs

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,16 @@
1-
use std::time::{Duration, Instant};
1+
use std::time::Duration;
22

3-
use backoff::backoff::Backoff as _;
4-
use backoff::ExponentialBackoff;
3+
use backon::ExponentialBuilder;
54

6-
use crate::constants::{
7-
DEFAULT_INITIAL_INTERVAL, DEFAULT_MAX_INTERVAL, DEFAULT_MULTIPLIER, DEFAULT_RANDOMIZATION,
8-
};
5+
use crate::constants::{DEFAULT_INITIAL_INTERVAL, DEFAULT_MAX_INTERVAL, DEFAULT_MULTIPLIER};
96

10-
/// Returns an ExponentialBackoff object instantianted with default values
11-
pub fn get_default_backoff() -> ExponentialBackoff {
12-
let mut eb = ExponentialBackoff {
13-
current_interval: Duration::from_millis(DEFAULT_INITIAL_INTERVAL),
14-
initial_interval: Duration::from_millis(DEFAULT_INITIAL_INTERVAL),
15-
randomization_factor: DEFAULT_RANDOMIZATION,
16-
multiplier: DEFAULT_MULTIPLIER,
17-
max_interval: Duration::from_millis(DEFAULT_MAX_INTERVAL),
18-
max_elapsed_time: None,
19-
clock: Default::default(),
20-
start_time: Instant::now(),
21-
};
22-
eb.reset();
23-
eb
7+
/// Returns an exponential backoff builder instantianted with default values
8+
pub fn get_default_backoff() -> ExponentialBuilder {
9+
ExponentialBuilder::new()
10+
.with_min_delay(Duration::from_millis(DEFAULT_INITIAL_INTERVAL))
11+
.with_max_delay(Duration::from_millis(DEFAULT_MAX_INTERVAL))
12+
.with_jitter()
13+
.with_factor(DEFAULT_MULTIPLIER)
2414
}
2515

2616
/// Trait for displaying duration-like in milliseconds

0 commit comments

Comments
 (0)