ref: Only obtain max retry count once (#2521)

With the current implementation, we attempt to retrieve the max retry count from the environment or from the ini file on every access, which could lead to the user being warned multiple times if an invalid value is supplied. Instead, we should only obtain this count once and store it in the `Config`

Author: szokeasaurusrex

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(),
+                self.config.max_retries(),
                 &[
                     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(),
+                self.api.config.max_retries(),
                 &[
                     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(),
+                self.api.config.max_retries(),
                 &[
                     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(),
+                self.api.config.max_retries(),
                 &[
                     http::HTTP_STATUS_502_BAD_GATEWAY,
                     http::HTTP_STATUS_503_SERVICE_UNAVAILABLE,
@@ -1408,7 +1408,7 @@ impl RegionSpecificApi<'_> {
         self.request(Method::Post, &path)?
             .with_form_data(form)?
             .with_retry(
-                self.api.api.config.get_max_retry_count(),
+                self.api.api.config.max_retries(),
                 &[http::HTTP_STATUS_507_INSUFFICIENT_STORAGE],
             )
             .progress_bar_mode(ProgressBarMode::Request)
@@ -1467,7 +1467,7 @@ impl RegionSpecificApi<'_> {
             .request(Method::Post, &path)?
             .with_form_data(form)?
             .with_retry(
-                self.api.api.config.get_max_retry_count(),
+                self.api.api.config.max_retries(),
                 &[
                     http::HTTP_STATUS_502_BAD_GATEWAY,
                     http::HTTP_STATUS_503_SERVICE_UNAVAILABLE,

src/config.rs

@@ -53,6 +53,7 @@ pub struct Config {
     cached_log_level: log::LevelFilter,
     cached_vcs_remote: String,
     cached_token_data: Option<AuthTokenPayload>,
+    max_retries: u32,
 }
 
 impl Config {
@@ -91,6 +92,7 @@ impl Config {
             cached_headers: get_default_headers(&ini),
             cached_log_level: get_default_log_level(&ini),
             cached_vcs_remote: get_default_vcs_remote(&ini),
+            max_retries: get_max_retries(&ini),
             ini,
             cached_token_data: token_embedded_data,
         }
@@ -468,27 +470,8 @@ impl Config {
     }
 
     /// 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
+    pub fn max_retries(&self) -> u32 {
+        self.max_retries
     }
 
     /// Return the DSN
@@ -539,6 +522,32 @@ impl Config {
     }
 }
 
+/// Obtains the maximum number of retries from the environment or the ini file.
+/// Environment variable takes precedence over the ini file. If neither is set,
+/// the default value is returned.
+fn get_max_retries(ini: &Ini) -> 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(ini) {
+        Ok(Some(val)) => return val,
+        Ok(None) => (),
+        Err(e) => {
+            warn!("Ignoring invalid {MAX_RETRIES_INI_KEY} ini key: {}", e);
+        }
+    };
+
+    DEFAULT_RETRIES
+}
+
 /// 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<Option<u32>> {
@@ -709,6 +718,7 @@ impl Clone for Config {
             cached_log_level: self.cached_log_level,
             cached_vcs_remote: self.cached_vcs_remote.clone(),
             cached_token_data: self.cached_token_data.clone(),
+            max_retries: self.max_retries,
         }
     }
 }
@@ -794,6 +804,7 @@ mod tests {
             cached_log_level: LevelFilter::Off,
             cached_vcs_remote: String::new(),
             cached_token_data: None,
+            max_retries: 0,
         };
 
         assert_eq!(