allan2 / dotenvy

A well-maintained fork of the dotenv crate
MIT License
668 stars 42 forks source link

v0.16 fetching variable error handling is inconsistent #120

Closed allan2 closed 1 week ago

allan2 commented 1 week ago

This applies to the v0.16 draft.

When using load_and_modify, variables are fetched using std::env::var, returning Result<String, VarError>.

When using the non-modifying load, the return type is HashMap<String, String>:. Fetching a variable uses the HashMap API, returning Option<String>. This requires the user to manually map to a missing error.

This inconsistency is an ergonomic problem.

allan2 commented 1 week ago

I am thinking to let the user handle the error themselves. This is a way to do it using a closure.

let env_map = EnvLoader::new().load()?;
let get_var = |k: &str| {
    env_map
        .get(k)
        .ok_or_else(|| MyError::VarNotSet(k.to_owned()))
};
let host = get_var("HOST")?;
let port = get_var("PORT")?;

I would prefer not to wrap HashMap<String, String> in a newtype. It's not ideal for the two APIs to differ in return signature, but the tradeoff doesn't seem worth it. VarError doesn't contain the key name anyways.

A relevant discussion

aidenfarley commented 1 week ago

@allan2 Maybe adding a v0.16 tag would be beneficial to marking issues that will be solved (or pertain to) items in v0.16?

allan2 commented 1 week ago

Error type is updated in the latest commits. EnvMap is now a newtype and Deref. dotenvy::var and EnvMap::var both return the same error type.