bug(load): Non-existent files fail silently

[Tbhesswebber]

It looks like the load function fails silently on all files when one file doesn't exist because there is no error handling in the parse function. It seems like the preferred functionality would be to either "yell" about the error from the catch block in the load function or to add error handling inside of the iteration on lines 45-47 of lib/dotenv-flow.js so that each file is read independently, any errors are caught and console.warned, and then the parsed file list is filtered down to just the files that can be read.

I would be more than happy to make a PR, but I wasn't sure if there is a preferred behavior.

https://github.com/kerimdzhanov/dotenv-flow/blob/38557217ac652272824c91e9b0f29d8aa56a7e7f/lib/dotenv-flow.js#L40-L48

[kerimdzhanov]

It seems to me that the right behavior to handle this kind of cases is something I proposed here in #41. Any thoughts?

[kerimdzhanov]

It's not a bug though. Given that we check all the files for existence before parsing, it should be considered as "unexpected" if something is failed. Therefore, in the context of .load and .parse functions it is correct behavior.

[Tbhesswebber]

Sorry for the lack of response - it has been a while since I had to even think about the issue because I worked around it with a separate call for each file.

Given that we check all the files for existence before parsing, it should be considered as "unexpected" if something is failed. Therefore, in the context of .load and .parse functions it is correct behavior.

Can you maybe clarify why being silent for unexpected usage is the correct behavior? I believe that my initial reporting was intended to point to the silent aspect of the failure being a bug rather than the failure itself being one, although I did provide an option for fixing it that prevents the silence by shifting the behavior.

[kerimdzhanov]

Hi @Tbhesswebber,

Yeah, sure, let me try…

The method .config() (being a basic entry point of initializing dotenv-flow), calls .listFiles() that returns a list of existing .env* (checking each with fs.fileExistSync() before listing). The list of existing files is then passed to .load(). Thus, .load() (being a kind of internal API exposed for programmatic usage), relies on the files' existence (and all kinds of accessible, readable, etc. criteria) for operating. So, if some of those criteria aren't met, the .parse() method throws an exception (considering it as an unexpected case) while .load() catches that error, warns about it, and returns the error within the .error property of the returned object.

So, yeah, I guess that your initial reporting was already fixed recently could you please check the latest version of dotenv-flow (v4.0.0) and let me know if it is still the issue?