Skip to content

Commit 4284d77

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 b7a8af5 commit 4284d77

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
}
@@ -468,26 +470,7 @@ impl Config {
468470
}
469471

470472
pub fn get_max_retry_count(&self) -> u32 {
471-
match max_retries_from_env() {
472-
Ok(Some(val)) => return val,
473-
Ok(None) => (),
474-
Err(e) => {
475-
warn!(
476-
"Ignoring invalid {MAX_RETRIES_ENV_VAR} environment variable: {}",
477-
e
478-
);
479-
}
480-
};
481-
482-
match max_retries_from_ini(&self.ini) {
483-
Ok(Some(val)) => return val,
484-
Ok(None) => (),
485-
Err(e) => {
486-
warn!("Ignoring invalid {MAX_RETRIES_INI_KEY} ini key: {}", e);
487-
}
488-
};
489-
490-
DEFAULT_RETRIES
473+
self.max_retries
491474
}
492475

493476
/// Return the DSN
@@ -538,6 +521,32 @@ impl Config {
538521
}
539522
}
540523

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

798809
assert_eq!(

0 commit comments

Comments
 (0)