(unintended?) npm@8 breaking change: return status different for npm outdatedΒ #40382
Copy link
Copy link
Closed
Labels
npmIssues and PRs related to the npm client dependency or the npm registry.Issues and PRs related to the npm client dependency or the npm registry.
Description
Version
v16.11.0
Platform
Microsoft Windows NT 10.0.19042.0 x64
Subsystem
No response
What steps will reproduce the bug?
require('child_process').execSync('npm outdated -g --depth=0')
How often does it reproduce? Is there a required condition?
every time
What is the expected behavior?
>node
Welcome to Node.js v16.10.0.
Type ".help" for more information.
> require('child_process').execSync('npm outdated -g --depth=0')
<Buffer 50 61 63 6b 61 67 65 20 20 20 20 20 20 20 20 20 20 20 20 20 43 75 72 72 65 6e 74 20 20 20 57 61 6e 74 65 64 20 20 20 4c 61 74 65 73 74 20 20 4c 6f 63 ... 216 more bytes>
What do you see instead?
>node
Welcome to Node.js v16.11.0.
Type ".help" for more information.
> require('child_process').execSync('npm outdated -g --depth=0')
Uncaught Error: Command failed: npm outdated -g --depth=0
at checkExecSyncError (node:child_process:826:11)
at Object.execSync (node:child_process:900:15) {
status: 1,
signal: null,
output: [
null,
<Buffer 50 61 63 6b 61 67 65 20 20 20 20 20 20 20 20 20 20 20 20 20 43 75 72 72 65 6e 74 20 20 20 57 61 6e 74 65 64 20 20 20 4c 61 74 65 73 74 20 20 4c 6f 63 ... 216 more bytes>,
<Buffer >
],
pid: 14996,
stdout: <Buffer 50 61 63 6b 61 67 65 20 20 20 20 20 20 20 20 20 20 20 20 20 43 75 72 72 65 6e 74 20 20 20 57 61 6e 74 65 64 20 20 20 4c 61 74 65 73 74 20 20 4c 6f 63 ... 216 more bytes>,
stderr: <Buffer >
}
Additional information
No response
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
npmIssues and PRs related to the npm client dependency or the npm registry.Issues and PRs related to the npm client dependency or the npm registry.
Activity
targos commentedon Oct 9, 2021
/cc @nodejs/npm
It seems that npm@8 now exits with code 1. npm@7 exited with code 0.
Mesteery commentedon Oct 9, 2021
nassau-t commentedon Oct 9, 2021
C:\temp>npm -v
8.0.0
C:\temp>npm outdated -g --depth=0
Package Current Wanted Latest Location Depended by
@types/node 16.10.2 16.10.3 16.10.3 node_modules/@types/node global
webpack-dev-server 4.3.0 4.3.1 4.3.1 node_modules/webpack-dev-server global
nassau-t commentedon Oct 9, 2021
Mesteery commentedon Oct 9, 2021
nassau-t commentedon Oct 9, 2021
mscdex commentedon Oct 9, 2021
Most likely the non-zero exit code is intended for use in scripting/automation to reliably know if there are outdated dependencies without having to try to parse the output, whose format could easily change.
Also you could look at it this way if you view all non-zero exit codes as "failures": npm failed to find no outdated dependencies.
nassau-t commentedon Oct 9, 2021
mscdex commentedon Oct 9, 2021
From my experience, non-zero exit codes for non-errors is very common in the *nix world. Even common utilities like grep do this (e.g. to signal if a match was made or not).
While Bash and others may have recommended meanings for various exit codes, there is no universal standard that everyone is required to follow. I would say the only commonly agreed upon meaning is for an exit status of
0, which can still be a bit vague but usually indicates some sort of positive ("success"/"good"/"ok") signal (again even in the case of grep it means one or more lines were matched, which can be viewed as a positive signal).Since the change in npm seems to have originated in a new major version (of npm) and npm still uses semver, it seems a sensible breaking change to me. Perhaps the course of action here would be to submit an issue on the npm CLI issue tracker to ask for this particular change to be noted in their v8.0.0 changelog.
nassau-t commentedon Oct 9, 2021
nassau-t commentedon Oct 9, 2021
Trott commentedon Oct 9, 2021
Off-topic stuff about grep return codes
Non-zero return status can indicate error but can also indicate other non-error failures. A well known example, and one that seems parallel to the case here, is
grep, which returns 0 if it finds a match, 1 if it doesn't find a match (failure, but not an error), and 2 for errors.I'm going to close this because it is in the wrong repository. You may open this issue in https://github.com/npm/cli if you want to argue that
npm outdatedmust return error codes according to your expectations. Node.js will use whatever npm hands it as the return status.targos commentedon Oct 9, 2021
I'd like to keep this open because npm 8 was rushed into Node.js 16 with this message:
Trott commentedon Oct 9, 2021
Works for me. I certainly agree it's a breaking change!
nassau-t commentedon Oct 10, 2021
nassau-t commentedon Oct 10, 2021
In parallel, I have opened an issue on npm site npm/cli#3871
[-]exec npm fails[/-][+](unintended?) npm@8 breaking change: return status different for `npm outdated`[/+]Trott commentedon Oct 10, 2021
I hid a bunch of comments here as off-topic. (@mscdex, I didn't hide yours because you're a Collaborator and subject to different rules, but if you feel like hiding or editing yours to keep this concise and focused, please do.) The issue at hand is that the return status is different between npm@7 and npm@8, and that that is apparently an unintended breaking change. Whether or not the changed behavior is correct is immaterial.
ljharb commentedon Oct 10, 2021
To clarify: this behavior did not actually change in npm v8; it changed in npm v7.24.2. In other words, it would have happened even if node had updated to npm 7 latest and not npm 8. npm v7.24.1 and below do not exit nonzero in this scenario.
nassau-t commentedon Oct 11, 2021
Created RFC npm/rfcs#473
Trott commentedon Oct 11, 2021
From npm/cli#3871 (comment):
As @nassau-t indicates in a previous comment, they've opened an RFC in the npm repository about it.
Given all this, I'm going to close this again. Because this wasn't a change in npm 8 (and therefore is not a result of #40369) and is not an issue with Node.js but with npm, the npm org is the right place for this to be discussed and for action (if any) to be taken.
@targos If you disagree and still think this should remain open, by all means, re-open this issue. Thanks!
nassau-t commentedon Oct 13, 2021
If someone else have to handle this exception...
BAD SOLUTION: A clear, smart program:
That fails, evidently.
GOOD SOLUTION: 2nd attempt, with a particular exception handled
More clear, as seen (ironic), but hey, it works!!!