Improve error handling in --json mode#4946
Conversation
There was a problem hiding this comment.
Thanks for the great PR! I have a few smaller comments, see below.
I'm a bit undecided how to handle the broken error message in the backup JSON output. It might be a good idea to maintain backwards compatibility with implementations that for example directly use the Go struct from the source code. Decoding the struct in Go would AFAICT result in a type error, when trying to decode {"error":"some message"} instead of {"error":{}}. So it might be better to use {"error":{"message":"some value"}} instead. That leads to a bit of not entirely necessary nesting, but would be fully backwards compatible and also provide room for adding an error code or similar. I'll have to think a bit about that.
[Edit]Parsing an old error message would fail: https://go.dev/play/p/qnXMavntl02 [/Edit]
e7c0346 to
33e8eb9
Compare
|
I've updated this PR to remove the metadata-error change (now in a separate PR) and address all your comments, I believe. Please re-review! Thanks |
|
It will probably take a few days until I can take a look. I'll first have to sort out the regressions from restic 0.17.0. |
|
No rush! I know I’m throwing a lot of PRs at ya, but there’s no time pressure from my end. Regressions come first! 😄 |
MichaelEischer
left a comment
There was a problem hiding this comment.
The changes look good so far. The only thing left to sort out are the linter errors and the replacement type for the error field.
Previously, an error JSON fragment would look like:
{"message_type": "error", "error": {}}
This is because encoding/json cannot marshal an error interface.
Instead, we now call .Error() to get the string value.
Previously, they were printed as freeform text. This also adds a ui.Terminal interface to make writing tests easier and also adds a few tests.
8e68abc to
7b3a3d6
Compare
|
OK should be ready again, and after noticing your comment about |
This keeps backwards compatibility with the previous empty structs. And maybe we'd want to put other fields into the inner struct later, rather than the outer message.
What does this PR change? What problem does it solve?
The main problem this solves are:
This addresses both of those, as well as a few fixes for issues I found along the way:
Was the change previously discussed in an issue or on the forum?
Closes #4944
Closes #4945
Checklist
changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.