volta-cli / rfcs

RFCs for changes to Volta
Other
17 stars 14 forks source link

autoshim exit codes #26

Closed dherman closed 5 years ago

dherman commented 5 years ago

I think it's worth discussing whether this bit of the exit codes RFC is wrong:

Additionally, shims must do their utmost to preserve the exit code supplied by the shimmed executable, only introducing a shim-specific exit code if the executable cannot be run at all. To this end, shims must return 0 when the shimmed executable does, even if the shim itself is not able to perform its work (for example, if a new shim cannot be created when installing a new executable via npm or yarn).

This feels wrong to me: if autoshimming fails, it's a "something is seriously wrong" situation, and you want that to be a hard error.

Given that npm and yarn only produce 0 or 1, it's not a loss of information to produce 1 if autoshimming fails. And I think it's fine to think of the shimmed yarn as having slightly more responsibility than the tool it delegates to. (I think swallowing the exit code of node would be more problematic, but we don't autoshim for node so that's not an issue.)

For event reporting, which runs in a background process and is really meant to be invisible to the Notion user, the exit code definitely shouldn't be affected. Although we'd want to figure out ways to report it (a notion command for detecting issues? hard error next time you run a command?).

charlespierce commented 5 years ago

We've removed Autoshimming, so I don't believe this is an issue any more.