Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] NPM 7.x broke the "--json" CLI parameter #2740

Closed
Tracked by #397 ...
octogonz opened this issue Feb 20, 2021 · 7 comments · Fixed by #5716
Closed
Tracked by #397 ...

[BUG] NPM 7.x broke the "--json" CLI parameter #2740

octogonz opened this issue Feb 20, 2021 · 7 comments · Fixed by #5716
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@octogonz
Copy link

The problem

NPM's --json option allows scripts to invoke an NPM command and receive machine-readable JSON that can be easily processed. In previous NPM versions, the JSON was written to STDOUT and all other messages were written to STDERR, making it easy for a script to extract the JSON.

With NPM 7.x, everything is mixed together on STDERR. This defeats the point of the --json feature, which was to make parsing easy. Instead, a script has to implement heuristics to guess which STDERR lines are JSON or not. Scripts developed for NPM 6 now fail because they are reading the wrong stream.

Steps To Reproduce:

  1. Correct behavior: With NPM 6.x, invoke this command:

    npm search --json
    

    The JSON output is written to STDOUT:

    {
      "error": {
        "code": null,
        "summary": "search must be called with arguments",
        "detail": ""
      }
    }

    The console messages are written to STDERR:

    npm ERR! search must be called with arguments
    npm ERR! A complete log of this run can be found in:
    
  2. Broken behavior: With NPM 7.5.4 (the latest version), invoke the same command. Both outputs are mixed together on STDERR. Nothing is written to STDOUT.

This regression appears to have been introduced with NPM 7.0.0.

Environment:

  • OS: All OS's
  • Node: 12.17.0
  • npm: 7.5.4
@octogonz octogonz added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Feb 20, 2021
@wraithgar wraithgar added Priority 2 secondary priority issue and removed Needs Triage needs review for next steps labels Mar 22, 2021
@RammasEchor
Copy link

Hi! I've been looking up this issue, and I would like to work on it.

@bdkjones
Copy link

Any progress on this? It's a big regression from 6.x for those of us who integrate npm into tooling. Thanks.

@octogonz
Copy link
Author

This was a significant regression for tools that rely on the NPM installation provided by Node.js: Since we can't assume that the developer has the latest Node.js, even if NPM eventually fixes this bug, it may be years before we can safely remove our workaround.

Looks like @RammasEchor made a PR but it was rejected.

snip

If it helps, here's the workaround that we used:

https://github.com/microsoft/rushstack/blob/b2aa54fa2a284e166529cdebbc3918bcd7dc7503/apps/rush-lib/src/logic/setup/SetupPackageRegistry.ts#L486

In brighter news, Node 16 will let you use Yarn without having to install it. yarn info has equivalent functionality, and Yarn's --json parameter is correctly implemented. 👍

@rossng
Copy link

rossng commented Sep 24, 2021

This regression is causing problems for atlassian/changesets users. For example: changesets/changesets#397

@bdkjones
Copy link

bdkjones commented Oct 7, 2021

Sounds like switching to yarn is the best approach. "Will get attention when we're freed up" sounds an awful lot like "Will sit in GitHub issues for seven years."

@lukekarrys
Copy link
Member

I did some digging and found that this was introduced in #2155 (which was to fix #2150).

It looks like npm ls --json (not sure about other commands yet) would print their own json blob and then the exit handler would print an additional json blob with the error. This caused unparseable json so the second blob was switched to stderr. Obviously, this caused regressions like this one.

A better solution I think would be to switch the main exit handler back to stdout and special case any commands that are already printing their own json output to not print more. I'm more confident (than I am about #2773) that this fix can come before any output refactor work we're doing. So hopefully it's less than seven years 😄

@Andarist
Copy link

The OP suggests that everything was previously sent to stderr - that doesn't sound right though. If I understand the issue correctly the change only affected JSON errors - they are now written to stderr where previously they were written to stdout. So the change is of a smaller scope than OP suggests. Can anyone confirm this? Don't take me wrong - this is still super annoying and affecting me directly (I'm a Changesets maintainer and this has caused problems in our publishing scripts), I just want to make sure that I understand the issue correctly so I can fix it properly on our end.

tjenkinson added a commit to video-dev/hls.js that referenced this issue Sep 10, 2022
tjenkinson added a commit to video-dev/hls.js that referenced this issue Sep 10, 2022
lukekarrys added a commit that referenced this issue Oct 19, 2022
This also adds a new output method `outputBuffer()` which will buffer
all output until it is flushed in the exit handler. This allows the exit
handler to catch any errors and append them to the output when in json
mode. This was necessary to not introduce a regression in the case of
#2150.

BREAKING CHANGE: `npm` now outputs some json errors on stdout.
Previously `npm` would output all json formatted errors on stderr,
making it difficult to parse as the stderr stream usually has logs
already written to it. In the future, `npm` will differentiate between
errors and crashes. Errors, such as `E404` and `ERESOLVE`, will be
handled and will continue to be output on stdout. In the case of a
crash, `npm` will log the error as usual but will not attempt to display
it as json, even in `--json` mode. Moving a case from the category of an
error to a crash will not be considered a breaking change. For more
information see npm/rfcs#482.

Closes #2740
Closes npm/statusboard#589
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants