Error variants should optionally include the found .env file Path

[andrewtc]

Hello, I have been using dotenvy in one of my personal projects. I'm building a command line app, so handling and reporting errors in a way that gives the user useful troubleshooting information is critical.

For example, if a .env file contains a parse error, I can't get away with simply printing something like this to stderr:

Error parsing line: 'LINE_WITHOUT_EQUALS', error at line index: 20

If I expect anyone to use my program more than once, I need the message displayed to the user to be more like this:

An error occurred while loading environment variables from the following file:

    path/to/current/dir/.env

The parse error occurred here, on line 1:

    LINE_WITHOUT_EQUALS
                       ^
                       Expected '=', but found end of line

Please correct the syntax error(s) in this file and try running the command again.

Empowering the user to fix the problem themselves results in an obviously better user experience than simply printing an error message. If I were to print the error message directly, the user would likely assume that an unexpected error had occurred, i.e. that my app is buggy or fragile. To be clear, I'm not saying that I would expect this level of detailed messaging at the dotenvy level. What I am saying is that as a developer, if an error occurs it is my responsibility to extract this sort of information from a dotenvy::Error and show something to the user along these lines.

Which leads me to my point. Right now, dotenvy::Error doesn't contain enough information to be this detailed with my error messaging.

Of course, there's a lot of information that could be added to Error. However, I believe the most important piece of missing information is the Path of the specific .env file that was loaded. For instance, dotenvy::dotenv() searches up the folder hierarchy from the current directory, which means that it may not be clear which .env file was located and parsed. If I get back a LineError, I have no idea which file was being parsed when the error occurred unless I run the search again myself.

More importantly, however, as a developer I shouldn't need to write code to infer which file was parsed. Which file is being parsed is important enough context that it should be included in the LineError.

I would love to see this information added to all Error variants that can be reported in a file/directory context. Since this would involve adding members to Error variants, this necessarily would be a breaking change. However, I feel that this improvement would be a significant enough to justify the version bump.

For anyone interested, I have a forked repo that includes one possible implementation. Please take a look at the diff here:
master...andrewtc:dotenvy:error_with_path

I tried to add the Path information while allowing for the possibility of adding more information to each Error variant in the future. That is the main reason why I split a couple of the variants out into separate sub-Errors (ParseError and IoError), although doing this also makes it clearer what each field represents.*

Both of the new Error structures I added contain an Option<PathBuf>. The reason this is optional is because it is possible to read environment variables from a stream (e.g. UnixStream in the examples). In this case, the Path information doesn't make sense. I would be open to an alternate design if anyone thinks the Option is annoying or that the non-File case warrants separate Error variants.

I would love to hear feedback from all of you on the design and implementation details. If anyone is interested in taking my changes, I'd be happy to open an issue and a PR.

* To prove my point, the usize member in LineError is being referred to in several places as line even though it is used like a character offset. Giving this a name (col) makes the intended usage clearer.

[allan2]

Thanks for the writeup.

I'm following rust-lang/rcfs #2885 which discusses including the file path in io::Error. A lot of the points there are also relevant to dotenvy.

I'd be curious to see benchmarks on how much cost there really is in including more info in the error case.

By the way, there are other discussions on this repo regarding an API redesign. I imagine that this proposed change would be a part of that larger, breaking release, which may be a ways away.

[allan2]

The upcoming v0.16 version now includes the path to the env file in the error message.