Windows: command output is not valid JSON: unexpected end of JSON input

[johnmaguire]

Describe the bug

error: forget: get snapshots for repo test-repo: command "C:\\Program Files/restic/restic-0.16.4.exe forget --json -o sftp.args=-oBatchMode=yes --tag plan:test-plan --group-by tag" failed: command output is not valid JSON: unexpected end of JSON input

To Reproduce
Steps to reproduce the behavior:

1. Create a new repo and plan.
2. Run a backup.
3. See error.

Expected behavior
Successful forget run.

Screenshots

Desktop (please complete the following information):

• OS: Windows
• Backrest 0.16.0

Additional context
This was previously fixed in #160 and I haven't seen this behavior on macOS, so I suspect this is a Windows-specific bug.

Update: I received the error many times, running against the same repo. When I changed the retention policy from the default schedule to a count of 1, it stopped erroring.

[garethgeorge]

Is this something you were able to reproduce any time you create a new plan? Or was this a plan that you inherited from an older version of backrest (and recently updated the backrest version)?

[garethgeorge]

Ah, I'm seeing now that this error is actually a crossplatform problem.

the defaults are poorly initialized at the moment. If none are set, then all cli flags are omitted so you get this error. Putting out a fix.

[garethgeorge]

Fixed in the latest release https://github.com/garethgeorge/backrest/releases/tag/v0.17.0

[johnmaguire]

@garethgeorge Just for my understanding, the issue is that when all "by time period" inputs are set to 0, restic returns an error instead of a JSON response? In other words, I hit this by skipping through and clicking "Submit" without reading the page. Meaning this is the fix, initializing these values? e96f403#diff-dea1ed7f9a3e7b84580826b2d1670e8fd5d4c89f73fed4d84fd5ea7b0b106221R490-R493

Do you think it's worth adding a validation step in case a user does zero these all out? It seems like a "nice to have" but not critical.

[garethgeorge]

Hey, yep! What's essentially happening here is we're calling restic forget without providing any flags at all for the policy.

Validation sounds like a good idea, this should probably be validation we actually do in the backend to block loading a config that's misconfigured e.g. here https://github.com/garethgeorge/backrest/blob/main/internal/config/validate.go#L102-L104 (and optionally in the UI -- but for I think just doing the backend check is probably safe for this).

I'll reopen this to track adding that check.

[garethgeorge]

Going ahead and marking this as resolved, the latest version should be handling tags and group-by correctly now.