Skip to content

Commit

Permalink
Make error more specific
Browse files Browse the repository at this point in the history
> Ed: This struct is only returned by one function, rather than being a general error struct for the whole binary. Should it have a more specific name than Error? eg BuildArgsError or similar?

Here's what that looks like. I'm okay with either. I think this is better if someone needs to add a different error in the future as this will guide them to make a new enum where appropriate.
  • Loading branch information
schneems committed Sep 11, 2023
1 parent 991adb4 commit abcf16d
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions buildpacks/ruby/src/bin/agentmon_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ where
}

#[derive(Debug, thiserror::Error, PartialEq)]
enum Error {
enum BuildArgsError {
#[error("{PORT} environment variable is not set")]
MissingPort,

Expand All @@ -108,16 +108,16 @@ enum Error {
///
/// - Environment variables: PORT or `HEROKU_METRICS_URL` are not set
/// - Environment variable DYNO starts with `run.`
fn build_args(env: &HashMap<String, String>) -> Result<Vec<String>, Error> {
fn build_args(env: &HashMap<String, String>) -> Result<Vec<String>, BuildArgsError> {
let mut args = Vec::new();
if env.get(DYNO).is_some_and(|value| value.starts_with("run.")) {
return Err(Error::RunDynoDetected);
return Err(BuildArgsError::RunDynoDetected);
}

if let Some(port) = env.get(PORT) {
args.push(format!("-statsd-addr=:{port}"));
} else {
return Err(Error::MissingPort);
return Err(BuildArgsError::MissingPort);
};

if env.get(AGENTMON_DEBUG).is_some_and(|value| value == "true") {
Expand All @@ -127,7 +127,7 @@ fn build_args(env: &HashMap<String, String>) -> Result<Vec<String>, Error> {
if let Some(url) = env.get(HEROKU_METRICS_URL) {
args.push(url.clone());
} else {
return Err(Error::MissingMetricsUrl);
return Err(BuildArgsError::MissingMetricsUrl);
};

Ok(args)
Expand All @@ -141,21 +141,21 @@ mod test {
fn missing_run_dyno() {
let result = build_args(&HashMap::from([("DYNO".to_string(), "run.1".to_string())]));

assert_eq!(result, Err(Error::RunDynoDetected));
assert_eq!(result, Err(BuildArgsError::RunDynoDetected));
}

#[test]
fn missing_metrics_url() {
let result = build_args(&HashMap::from([("PORT".to_string(), "123".to_string())]));

assert_eq!(result, Err(Error::MissingMetricsUrl));
assert_eq!(result, Err(BuildArgsError::MissingMetricsUrl));
}

#[test]
fn missing_port() {
let result = build_args(&HashMap::new());

assert_eq!(result, Err(Error::MissingPort));
assert_eq!(result, Err(BuildArgsError::MissingPort));
}

#[test]
Expand Down

0 comments on commit abcf16d

Please sign in to comment.