Open daniel-rikowski opened 2 months ago
garethgeorge
commented
2 months ago Hey, I think of this as intended behavior when a repo is append only -- Backrest should let you know that the commands it's running are failing!
For append only you should be setting retention policy to "none" so backrest won't try to run any forget operations on your behalf, you'll need to script forgets elsewhere.
daniel-rikowski
commented
1 month ago I agree, this is a misconfigured backup job and Backrest should forward the resulting restic errors.
My issue was aimed at two other aspects:
1. Not all errors from restic are of the same severity.
IMHO a failed backup is catastrophic, a failed forget not so much. But I think this is debatable. Also providing handling for each and every error restic might throw, while also considering the context from which backrest called that restic command, would be a huge long term liability and a maintenance nightmare. So I'd like to retract that request 🥲
2. An error by restic should not be displayed as a JSON parse error.
The fact that Backrest can't parse the output of restic is more of an implementation detail, and not the actual error from the user's perspective.
This is not so much a problem with the full error output, but with the small "toast" message: It only shows the first line of the whole error message (as far a I can remember) This contains the whole command line and the JSON error, while the actual problem ("Remove(<snapshot/64806688d1>) failed: unexpected HTTP response (403): 403 Forbidden") is not readily visible.
When I first tried out Backrest, and I did a lot of dumb stuff, I encountered them quite frequently, and it took me while to figure out the actual error.
Perhaps backrest can differentiate between restic output/error modes, by...
Stderr (and not Stdout), then... (**)... show the first one or two lines from the restic output as the error message.
I believe this would be much more helpful to the users. But perhaps it's not that simple to implement.
My Go knowlededge is non-existant, so I can't make a PR...
(**) I'm not sure restic actually does this, though
(*) Some commands have specialized error statuses, perhaps Backrest can do something with it:
restic backup --help
...
Exit status is 0 if the command was successful.
Exit status is 1 if there was a fatal error (no snapshot created).
Exit status is 3 if some source data could not be read (incomplete snapshot created).
Exit status is 10 if the repository does not exist.
Exit status is 11 if the repository is already locked.
restic forget --help
...
Exit status is 0 if the command was successful.
Exit status is 1 if there was any error.
Exit status is 10 if the repository does not exist.
Exit status is 11 if the repository is already locked.
Is your feature request related to a problem? Please describe.
When using an append-only REST respository restic is unable - of course - to delete previous snapshots.
Unfortunately Backrest is unable to handle the non-JSON response and shows error messages and a red exclamation mark.
Describe the solution you'd like
I'd like Backrest to have some special handling for this situation (i.e. calling "forget" on a REST repo and receiving a non-JSON string with the substring "unexpected HTTP response (403)" )
Perhaps some message instead: "Could not remove snapshots. Is this an append-only repository?" or something similar.
I'd also like the red exclamation mark to become more differentiated. Since the backups itself are successful, a warning-like icon would be more appropriate, since this is an error only from Backrest's perspective.
Additional context
For your convenience the output of such an error: