search

a-scie / jump

A Self Contained Interpreted Executable Launcher
Apache License 2.0
48 stars 7 forks source link

.env loading fails partially and silently on invalid files #163

Closed huonw closed 10 months ago

huonw commented 10 months ago

The current use of dotenvy for applying .env files results in:

  1. errors being silently ignored (e.g. lines that can't be parsed), which is very confusing for users of a scie-jump packaged binary, like scie-pants
  2. (potentially not a problem) a .env file being partially applied, where all lines before the first error are applied, and all lines (even valid ones) after the first error are ignored (due to dotenvy's immediate bail out on error https://github.com/allan2/dotenvy/blob/08e35eea693ca12c4038a226bae2e3144445ba69/dotenv/src/iter.rs#L33)

https://github.com/a-scie/jump/blob/f44ca18f9c6e8a02f8fab84f6e22c8887b5a3267/jump/src/lib.rs#L181-L186

This was observed in practice in https://github.com/pantsbuild/scie-pants/issues/307.

jsirois commented 10 months ago

Unfotunately, the dotenvy API does not distinguish between an error due to no .env files found (which scie-jump would not want to treat as an error) and an error parsing the file found; so I think this will require not using dotenvy to implement .env support or else getting an enhancement through that project to support splitting finding the appropriate .env file to load (if any) and actually loading it. Of course yet another option is to do .env file discovery ourselves and just use dotenvy for parsing / export.

jsirois commented 10 months ago

Aha - I'm dumb: https://docs.rs/dotenvy/0.15.7/dotenvy/fn.from_filename.html

jsirois commented 10 months ago

I'm dumber - the returned error type is rich enough to distinguish.

jsirois commented 10 months ago

Alright, the fix is now released in 0.13.2: https://github.com/a-scie/jump/releases/tag/v0.13.2