Skip to content
New issue

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

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

Already on GitHub? # to your account

[BUG] --json outputs errors to stdout instead of stderr #2150

Closed
raineorshine opened this issue Nov 9, 2020 · 0 comments
Closed

[BUG] --json outputs errors to stdout instead of stderr #2150

raineorshine opened this issue Nov 9, 2020 · 0 comments
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release

Comments

@raineorshine
Copy link

raineorshine commented Nov 9, 2020

Current Behavior:

Running npm ls --json with missing dependencies outputs the error to stdout instead of stderr. This creates two top-level JSON objects, making it unparseable.

This is a regression from v6.

May be related to: #1109

Expected Behavior:

Normal output should print to stdout, errors should print to stderr.

i.e.

stdout:

{
  "name": "temp",
  "problems": [
    "missing: foo@1.0.0, required by temp@"
  ],
  "dependencies": {
    "foo": {
      "required": "1.0.0",
      "missing": true,
      "problems": [
        "missing: foo@1.0.0, required by temp@"
      ]
    }
  }
}

stderr:

{
  "error": {
    "code": "ELSPROBLEMS",
    "summary": "missing: foo@1.0.0, required by temp@",
    "detail": ""
  }
}

Steps To Reproduce:

For the life of me I can't figure out why this test passes, as it clearly asserts the expected stdout.

That said, I can reproduce the issue running the cli manually on a simple package.json:

$ mkdir issue-2150 && cd issue-2150
$ echo '{ "dependencies": { "foo": "1.0.0" } }' > package.json
$ npm ls --json 2>/dev/null

2>/dev/null makes it clear that the following output is all on stdout:

{
  "name": "temp",
  "problems": [
    "missing: foo@1.0.0, required by temp@"
  ],
  "dependencies": {
    "foo": {
      "required": "1.0.0",
      "missing": true,
      "problems": [
        "missing: foo@1.0.0, required by temp@"
      ]
    }
  }
}
{
  "error": {
    "code": "ELSPROBLEMS",
    "summary": "missing: foo@1.0.0, required by temp@",
    "detail": ""
  }
}

Environment:

OS: 10.14.6
Node: 15.1.0
npm: 7.0.9

@raineorshine raineorshine 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 Nov 9, 2020
isaacs added a commit that referenced this issue Nov 11, 2020
darcyclarke pushed a commit that referenced this issue Nov 13, 2020
@isaacs isaacs closed this as completed in 1dbf0f9 Nov 13, 2020
@darcyclarke darcyclarke added this to the OSS - Sprint 19 milestone Nov 16, 2020
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
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
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
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
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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps 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.

2 participants