API._streaming_download() does not handle HTTP 416 Range Not Satisfiable errors

[cfm]

Description

API._streaming_download() chokes on HTTP 416 Range Not Satisfiable errors, because the JSON error object may be received in the first chunk of any request, including any retry, not only at the beginning of the accumulated response.

Steps to Reproduce

--- a/client/securedrop_client/sdk/__init__.py
+++ b/client/securedrop_client/sdk/__init__.py
@@ -153,7 +153,7 @@ class API:
             try:
                 # Update the range request if we're retrying
                 if retry > 0:
-                    data["headers"]["Range"] = f"bytes={bytes_written}-"
+                    data["headers"]["Range"] = f"bytes={bytes_written}-{bytes_written-1}"
                     logger.debug(f"Retry {retry}, range: {bytes_written}-")
 
                 # Serialize the data to send

Expected Behavior

The SDK handles the error gracefully.

Actual Behavior

        filepath = os.path.join(path, submission.filename)
>       Path(filepath).write_bytes(response.contents)
E       AttributeError: 'NoneType' object has no attribute 'contents'

securedrop_client/sdk/__init__.py:699: AttributeError

[legoktm]

Nice catch. Is this just applicable to 416 or could this be literally any error if e.g. the server returns a 504?

Ideally this would've been a type error caught by mypy...

[cfm]

Yes, you're right: any retry-level error fails this way, and we don't handle HTTP 416 anywhere.

I was going to ask you about mocking Popen.stdin for the purpose of mangling a Range header to get a real HTTP 416 cassette, but I think I'll give pytest-subprocess a try first.

[cfm]

Clarifying from backlog-pruning: I'd like to finish this next week for v0.14. Otherwise we should defer it to v0.15 or a v0.14.x.