search

garethgeorge / backrest

Backrest is a web UI and orchestrator for restic backup.
GNU General Public License v3.0
787 stars 31 forks source link

Adding --read-data-subset breaks backups #307

Closed Thinkscape closed 1 month ago

Thinkscape commented 1 month ago

Describe the bug

Adding --read-data-subset= flag breaks the plan. I presume it's got something to do with escaping the flags when passing them through, or maybe it just doesn't start and the error is a red herring b/c there's no status json being generated.

To Reproduce

Steps to reproduce the behavior:

  1. Create a plan
  2. Add the flag --read-data-subset="5%"
  3. Start backup

Expected behavior

Backup runs.

Actual

error: failed to backup: command "/bin/restic-0.16.4 backup --json --exclude-caches /volume1/data -o sftp.args=-oBatchMode=yes --exclude-caches --tag plan:data --tag created-by:nas.local --exclude #Recycle --parent 79973b65370039ebba966ad0644590a15d3b679a4ed47de7a2f7efd47859076a --read-data-subset=5%" failed: exit code -1: backup failed
processing command output: command output was not JSON: invalid character 'u' looking for beginning of value

Note: I don't believe the error is the actual root cause. I'd expect the actual STDERR from restic would show up here to be able to debug what's causing the problem. Could we make backrest emit the actual error?

Platform Info

garethgeorge commented 1 month ago

Hey, click the “view logs” button next to your failed operation to get the root cause. The flag you’re passing looks like one for restic check — it is probably not working with the backup command.

You’re right that the error handling isn’t behaving correctly right now (a more detailed message should be included). Ive already written a fix for this for the next update :)

garethgeorge commented 1 month ago

Marking as resolved, error handling is improved in 1.1.0. Feel free to ping if you're still having issues.

Thinkscape commented 1 month ago

The flag you’re passing looks like one for restic check — it is probably not working with the backup command.

Gotcha. Note: Current UI doesn't allow specifying flags for individual commands, so I inferred that I'm allowed to provide flags for all commands. We should consider that going in the future, as restic adds more flags, and backrest has better coverage of restic functionality and backups' maintenance.