I think that the std::env::VarError type holds too little information. If the program errors on not being able to find an environmental variable, no useful information is given to the user.
std::env::var takes the key as its parameter. If it returns VarError, you already have the key, so it seems superfluous to duplicate it in VarError.
I don't think it's entirely superfluous. If you just ? the error, and it gets printed, it'd be nice if it prints the variable name, without having to .with_context(|| format!("Error getting variable {varname}"))?
This seems like it has all the same tradeoffs as having file opens include the filename in errors: it adds overhead, but it makes error reporting better and easier.
5 Likes
Unfortunately, this would be a breaking change, because VarError is a pub enum and not #[non_exhaustive], so adding a field to VarError::NotPresent or adding a new variant would break existing matches.
On the other hand, there is reason to consider overhauling the environment access API due to memory safety issues:
If that were done, a new error type could be included as part of the new API. The existing VarError is not Copy and already contains an OsString, so this would not remove any of its properties; but the new interface would either need a generic error type, or to copy the error, since the signature of env::var is
pub fn var<K: AsRef<OsStr>>(key: K) -> Result<String, VarError>
and the K must be either moved as-is into the error value, or copied into an OsString. The advantage of copying is simplicity and not exposing implementation-detail argument types, but it would be wasteful in the case where code is checking for the existence of environment variables and not stopping on error (e.g. imagine some moderately-frequently-called code which checks for an environment variable every time).
2 Likes
I think this breaks the zero-overhead principle. I don't need the path in the error if I'm the source of the error (as I have what I passed in available), but I do if I generated the path based on other information passed in (that the caller may not have access to). Basically, I'd rather explicitly add the context (with the explicit allocation if necessary) than have to think about such low-level things being inaccessible without considering allocations.
6 Likes
This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.