Reply "insufficient storage" on disk full or over-quota

Enrico204 commented 3 years ago

What is the purpose of this change? What does it change?

This pull request will change the current behavior on disk-related errors:

Previously both were 400 "Bad request"

[X] I have enabled maintainer edits for this PR
[x] I have added tests for all changes in this PR
[x] I have added documentation for the changes (in the manual)
[x] There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
[X] I have run gofmt on the code in all commits
[X] All commit messages are formatted in the same style as the other commits in the repo
[x] I'm done, this Pull Request is ready for review

wojas commented 3 years ago

Thanks! The patch looks good, but the tests need to be updated, because they check for a 400.

Enrico204 commented 3 years ago

To fix all tests, I had to:

Add a variable for "file content does not match hash" error so I can distinguish between internal errors and bad request (to send a content that doesn't match the hash is bad request). Relevant lines:
Change the error used in TestAbortedRequest function: now the function is using a "plausible" and declared error. The reason behind this change is because the h.internalServerError was crashing the server (due to panic() and PanicOnError == true) on stream reading error. Now the code detects whether the error is a network-related error (like unexpected EOF) and it use the correct HTTP status (400 Bad Request). Relevant change:
- https://github.com/restic/rest-server/pull/160/files#diff-f3352cbd560f9467e98af5ecd129c7d1f43917302653fe5ceafbd8bc857f83c7R399
- https://github.com/restic/rest-server/pull/160/files#diff-65e557a13cfc70927d6259fdf0113fff370ecddebb867153d67ef9d0877d8db4R612-R628

Tests should be OK. I also added some comments to clarify each if branch :-)