Skip to content

Commit c9f2fee

Browse files
ref: Only obtain max retry count once
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`
1 parent 7ee7cad commit c9f2fee

File tree

1 file changed

+31
-20
lines changed

1 file changed

+31
-20
lines changed

src/config.rs

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ pub struct Config {
5353
cached_log_level: log::LevelFilter,
5454
cached_vcs_remote: String,
5555
cached_token_data: Option<AuthTokenPayload>,
56+
max_retries: u32,
5657
}
5758

5859
impl Config {
@@ -91,6 +92,7 @@ impl Config {
9192
cached_headers: get_default_headers(&ini),
9293
cached_log_level: get_default_log_level(&ini),
9394
cached_vcs_remote: get_default_vcs_remote(&ini),
95+
max_retries: obtain_max_retry_count(&ini),
9496
ini,
9597
cached_token_data: token_embedded_data,
9698
}
@@ -469,26 +471,7 @@ impl Config {
469471

470472
/// Returns the configured maximum number of retries for failed HTTP requests.
471473
pub fn get_max_retry_count(&self) -> u32 {
472-
match max_retries_from_env() {
473-
Ok(Some(val)) => return val,
474-
Ok(None) => (),
475-
Err(e) => {
476-
warn!(
477-
"Ignoring invalid {MAX_RETRIES_ENV_VAR} environment variable: {}",
478-
e
479-
);
480-
}
481-
};
482-
483-
match max_retries_from_ini(&self.ini) {
484-
Ok(Some(val)) => return val,
485-
Ok(None) => (),
486-
Err(e) => {
487-
warn!("Ignoring invalid {MAX_RETRIES_INI_KEY} ini key: {}", e);
488-
}
489-
};
490-
491-
DEFAULT_RETRIES
474+
self.max_retries
492475
}
493476

494477
/// Return the DSN
@@ -539,6 +522,32 @@ impl Config {
539522
}
540523
}
541524

525+
/// Obtains the maximum number of retries from the environment or the ini file.
526+
/// Environment variable takes precedence over the ini file. If neither is set,
527+
/// the default value is returned.
528+
fn obtain_max_retry_count(ini: &Ini) -> u32 {
529+
match max_retries_from_env() {
530+
Ok(Some(val)) => return val,
531+
Ok(None) => (),
532+
Err(e) => {
533+
warn!(
534+
"Ignoring invalid {MAX_RETRIES_ENV_VAR} environment variable: {}",
535+
e
536+
);
537+
}
538+
};
539+
540+
match max_retries_from_ini(ini) {
541+
Ok(Some(val)) => return val,
542+
Ok(None) => (),
543+
Err(e) => {
544+
warn!("Ignoring invalid {MAX_RETRIES_INI_KEY} ini key: {}", e);
545+
}
546+
};
547+
548+
DEFAULT_RETRIES
549+
}
550+
542551
/// Computes the maximum number of retries from the `SENTRY_HTTP_MAX_RETRIES` environment variable.
543552
/// Returns `Ok(None)` if the environment variable is not set, other errors are returned as is.
544553
fn max_retries_from_env() -> Result<Option<u32>> {
@@ -709,6 +718,7 @@ impl Clone for Config {
709718
cached_log_level: self.cached_log_level,
710719
cached_vcs_remote: self.cached_vcs_remote.clone(),
711720
cached_token_data: self.cached_token_data.clone(),
721+
max_retries: self.max_retries,
712722
}
713723
}
714724
}
@@ -794,6 +804,7 @@ mod tests {
794804
cached_log_level: LevelFilter::Off,
795805
cached_vcs_remote: String::new(),
796806
cached_token_data: None,
807+
max_retries: 0,
797808
};
798809

799810
assert_eq!(

0 commit comments

Comments
 (0)