diff --git a/src/api/errors/api_error.rs b/src/api/errors/api_error.rs index bbcfb93add..775f46d509 100644 --- a/src/api/errors/api_error.rs +++ b/src/api/errors/api_error.rs @@ -44,8 +44,6 @@ pub(in crate::api) enum ApiErrorKind { "DSN missing. Please set the `SENTRY_DSN` environment variable to your project's DSN." )] DsnMissing, - #[error("Error preparing request")] - ErrorPreparingRequest, } impl fmt::Display for ApiError { diff --git a/src/api/mod.rs b/src/api/mod.rs index 0cbaa3ad38..6cfc7454fb 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -396,7 +396,7 @@ impl Api { .request(Method::Post, url, None)? .with_form_data(form)? .with_retry( - self.config.get_max_retry_count().unwrap(), + self.config.get_max_retry_count(), &[ http::HTTP_STATUS_502_BAD_GATEWAY, http::HTTP_STATUS_503_SERVICE_UNAVAILABLE, @@ -968,7 +968,7 @@ impl<'a> AuthenticatedApi<'a> { self.request(Method::Post, &url)? .with_json_body(request)? .with_retry( - self.api.config.get_max_retry_count().unwrap(), + self.api.config.get_max_retry_count(), &[ http::HTTP_STATUS_502_BAD_GATEWAY, http::HTTP_STATUS_503_SERVICE_UNAVAILABLE, @@ -1001,7 +1001,7 @@ impl<'a> AuthenticatedApi<'a> { dist: None, })? .with_retry( - self.api.config.get_max_retry_count().unwrap(), + self.api.config.get_max_retry_count(), &[ http::HTTP_STATUS_502_BAD_GATEWAY, http::HTTP_STATUS_503_SERVICE_UNAVAILABLE, @@ -1032,7 +1032,7 @@ impl<'a> AuthenticatedApi<'a> { dist, })? .with_retry( - self.api.config.get_max_retry_count().unwrap(), + self.api.config.get_max_retry_count(), &[ http::HTTP_STATUS_502_BAD_GATEWAY, http::HTTP_STATUS_503_SERVICE_UNAVAILABLE, @@ -1408,12 +1408,7 @@ impl RegionSpecificApi<'_> { self.request(Method::Post, &path)? .with_form_data(form)? .with_retry( - self.api.api.config.get_max_retry_count().map_err(|e| { - ApiError::with_source( - ApiErrorKind::ErrorPreparingRequest, - e.context("Could not parse retry count"), - ) - })?, + self.api.api.config.get_max_retry_count(), &[http::HTTP_STATUS_507_INSUFFICIENT_STORAGE], ) .progress_bar_mode(ProgressBarMode::Request) @@ -1472,7 +1467,7 @@ impl RegionSpecificApi<'_> { .request(Method::Post, &path)? .with_form_data(form)? .with_retry( - self.api.api.config.get_max_retry_count().unwrap(), + self.api.api.config.get_max_retry_count(), &[ http::HTTP_STATUS_502_BAD_GATEWAY, http::HTTP_STATUS_503_SERVICE_UNAVAILABLE, diff --git a/src/config.rs b/src/config.rs index 681d52a262..db5125a853 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,8 +1,10 @@ //! This module implements config access. use std::env; +use std::env::VarError; use std::fs; use std::fs::OpenOptions; use std::io; +use std::num::ParseIntError; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -26,6 +28,9 @@ use crate::utils::http::is_absolute_url; #[cfg(target_os = "macos")] use crate::utils::xcode; +const MAX_RETRIES_ENV_VAR: &str = "SENTRY_HTTP_MAX_RETRIES"; +const MAX_RETRIES_INI_KEY: &str = "max_retries"; + /// Represents the auth information #[derive(Debug, Clone)] pub enum Auth { @@ -462,14 +467,28 @@ impl Config { .unwrap_or(DEFAULT_MAX_DIF_ITEM_SIZE) } - pub fn get_max_retry_count(&self) -> Result { - if env::var_os("SENTRY_HTTP_MAX_RETRIES").is_some() { - Ok(env::var("SENTRY_HTTP_MAX_RETRIES")?.parse()?) - } else if let Some(val) = self.ini.get_from(Some("http"), "max_retries") { - Ok(val.parse()?) - } else { - Ok(DEFAULT_RETRIES) - } + /// Returns the configured maximum number of retries for failed HTTP requests. + pub fn get_max_retry_count(&self) -> u32 { + match max_retries_from_env() { + Ok(Some(val)) => return val, + Ok(None) => (), + Err(e) => { + warn!( + "Ignoring invalid {MAX_RETRIES_ENV_VAR} environment variable: {}", + e + ); + } + }; + + match max_retries_from_ini(&self.ini) { + Ok(Some(val)) => return val, + Ok(None) => (), + Err(e) => { + warn!("Ignoring invalid {MAX_RETRIES_INI_KEY} ini key: {}", e); + } + }; + + DEFAULT_RETRIES } /// Return the DSN @@ -520,6 +539,24 @@ impl Config { } } +/// Computes the maximum number of retries from the `SENTRY_HTTP_MAX_RETRIES` environment variable. +/// Returns `Ok(None)` if the environment variable is not set, other errors are returned as is. +fn max_retries_from_env() -> Result> { + match env::var(MAX_RETRIES_ENV_VAR) { + Ok(val) => Ok(Some(val.parse()?)), + Err(VarError::NotPresent) => Ok(None), + Err(e) => Err(e.into()), + } +} + +/// Computes the maximum number of retries from the `max_retries` ini key. +/// Returns `Ok(None)` if the key is not set, other errors are returned as is. +fn max_retries_from_ini(ini: &Ini) -> Result, ParseIntError> { + ini.get_from(Some("http"), MAX_RETRIES_INI_KEY) + .map(|val| val.parse()) + .transpose() +} + fn warn_about_conflicting_urls(token_url: &str, manually_configured_url: Option<&str>) { if let Some(manually_configured_url) = manually_configured_url { if manually_configured_url != token_url {