npm install "unmet dependencies" should be an error, triggering an error exit code

[markstos]

Currently npm install will print a warning if something fails due to unmet dependencies but will still "succeed" in the bash sense: returning a successful exit code.

Here's an example bash script to show the policy of exit-with-success-with-broken-dependencies can be problematic:

# Exit immediately if any command fails
set -o errexit
npm install
rsync ./ user@remotehost:/home/user/production

It would appear from the script that the code would only deployed if npm install succeeds, but with current behavior, missing dependencies could be shipped.

I see almost 100 issues in this queue that related to "unmet dependencies" and none of the ones I looked conveyed that the users thought the npm install was successful with unmet dependencies.

I can potentially contribute a pull request for the behavior change but wanted to check-in first to see if there were causes where current behavior of succeed-with-unmet-dependencies is desirable.

[othiym23]

Some classes of "unmet dependencies" are definitely errors, but some are just warnings, and shouldn't raise an error exit code. For instance, when upgrading from npm@1 to npm@2, installing global packages can result in a long list of warnings, because there are global packages with installed dependencies that have prerelease versions, which (due to changes in how semver ranges match in semver@4) are no longer satisfactory matches for their parents' dependency ranges.

These packages still work the same as they did before the upgrade, but due to the changed rules in semver, npm ls and npm install will print errors. I don't think this should halt later tasks. The dependency tree is in a suboptimal state, but it's not necessarily wrong, and there hasn't necessarily been a failure, there's just one or more inconsistencies. And in some of these cases, bailing out with an error gets in the way of your ability to fix problems.

So, if you want to take a whack at changing how npm deals with unmet dependency errors, your first job is to figure out which of these errors are really deserving of error status (and maybe change the rest to warnings). The next task is to look at how changing this behavior would affect people using npm in scripts, and try to figure out if there are situations where this would cause more problems than it would solve (which is what I try to do when reviewing these patches). Then take a stab at it! Tests for something like this are a requirement, but if they turn out to be tough to write (they often are), I'm happy to help out when I have time.

[markstos]

Thanks for the detailed explanation. I see now that perfectly declaring "unmet dependencies" as a "failure" or "success" could be hard.

What about flag: --fail-on-unmet-dependencies?

In cases like ours, there is no question: Any time unmet dependencies happens, it's unexpected and we'd like to fail-- we would explect npm install to fail. Currently we workaround this by capturing npm's output to a file, then grep'ing the file for 'unmet dependency', and testing exit code of grep.

Alternatively, perhaps one or two specific sub-cases of "unmet dependencies" could be dealt with:

1. If a package fails to download (perhaps due to temporary networking issue), have an option to consider that a failure.
2. If there is no version of a module dependency (compared to one that just doesn't met the semvar criteria), consider that a failure.

The network-failure case might be hard to test for if a module could legitimately be not-found at at network location but ultimately be found somewhere. The absolutely-no-version-found case seems fairly more clear cut.

[othiym23]

I agree that both 1. and i. should be hard enough failures to halt installation, and this may in fact be how things work in the multi-stage branch that will become npm@3 as soon as it's ready. @iarna?

[markstos]

Oops, I meant "1" and "2". Thanks for the feedback!

[d5115r]

Complete noob but running into this all day trying to deploy parsoid/visual editor.

[rochapablo]

Same here...

heroku/heroku-buildpack-nodejs#288

[npm-robot]

We're closing this issue as it has gone thirty days without activity. In our experience if an issue has gone thirty days without any activity then it's unlikely to be addressed. In the case of bug reports, often the underlying issue will be addressed but finding related issues is quite difficult and often incomplete.

If this was a bug report and it is still relevant then we encourage you to open it again as a new issue. If this was a feature request then you should feel free to open it again, or even better open a PR.

For more information about our new issue aging policies and why we've instituted them please see our blog post.