Format exit errors as JSON if requested#4952
Conversation
f7343e7 to
194ef1a
Compare
MichaelEischer
left a comment
There was a problem hiding this comment.
Fine with me in general. I just have a few nitpicks, see below.
| Output formats | ||
| -------------- | ||
|
|
||
| Currently only the output on ``stdout`` is JSON formatted. Errors printed on ``stderr`` |
There was a problem hiding this comment.
Why did you remove this remark? I think we still want to keep a remark that some stderr output is still text and not JSON.
There was a problem hiding this comment.
I removed it because I felt like the errors-as-JSON situation was so much better that it didn't deserve a big warning. But maybe I over-corrected. How about the current text:
Commands print their main JSON output on ``stdout``.
The generated JSON output uses one of the following two formats.
.. note::
Not all messages and errors have been converted to JSON yet.
Feel free to submit a pull request!
That note mirrors an earlier one in the doc about some commands not supporting --json at all.
(I also emphasized that some error messages print to stderr a few more places in the docs.)
doc/075_scripting.rst
Outdated
| Exit errors | ||
| ----------- | ||
|
|
||
| Fatal errors will result in a final JSON message before the process exits. |
There was a problem hiding this comment.
It might be interesting to add a remark that JSON output cannot be guaranteed if parsing the CLI arguments failed.
There was a problem hiding this comment.
I added this comment:
.. note::
Some errors cannot be caught and reported this way,
such as Go runtime errors or command line parsing errors.
What does this PR change? What problem does it solve?
When using
--json, exit errors are now formatted as JSON instead of freeform text. And the exit code is included in the JSON blob, for the convenience of any consumers (so they don't have to wait for the exit code and then tie it together with the message).While I was testing, I also noticed that the
versioncommand didn't use amessage_typefield in JSON mode - I figured I'd toss one in there for consistency with other messages. If that is undesired, I'm happy to take it out of this PR.Was the change previously discussed in an issue or on the forum?
Closes #4948
Checklist
[ ] I have added tests for all code changes.(this felt hard to do with the exit call, unless I refactored things a bunch - but I did manually test this)changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.