core: remove NO_ERROR from default responses #7358
Conversation
There was a problem hiding this comment.
nice! This will be great.
Does this completely close #6336, though? e.g. do we give a bad exit code when there is a runtimeError? I can't remember where we fell on that.
|
Added |
|
sorry, I wasn't trying to tell you you needed to add that to this PR, just that we might not be done with that issue yet :) |
There was a problem hiding this comment.
I was trying to add a
exit code 1when there was no LHR
as you said, there are valid cases that don't output an LHR, and I don't think it's actually possible to get no LHR from a regular return in any other case...every other path there should be a thrown error (already caught in run.js). So just adding a check for a runtimeError should be enough.
But maybe defer to another PR so we can discuss/land that separately?
lighthouse-cli/index.js
Outdated
| require('./bin.js').begin().catch(err => { | ||
| require('./bin.js').begin().then((lhr) => { | ||
| // Exit with non-zero exit code if LHR contains runtime error. | ||
| if (lhr && lhr.lhr.runtimeError) { |
There was a problem hiding this comment.
ideally we can handle this in run.js with the other runtime error/exiting code
|
Rolled back exit code changes for follow up PR. Added test. Ready for re-review 😄 |
Summary
Removes NO_ERROR codes from default responses that contain no errors.
From some initial testing I did, this should have no effect on LR.
Related Issues/PRs
part of: #6336