Stream reset on retry

[fboismenu]

Hello there,

I am using the library, indirectly via python-storage, in maybe an unorthodox way: I am trying to implement a download timeout via a custom http adapter and response.
So my response object is throwing pro-actively timeouts (common.InvalidResponse with http.client.REQUEST_TIMEOUT),
which is forcibly triggering the retry logic in the Download class:

google-resumable-media-python/google/resumable_media/requests/download.py

Line 164 in d082772

def retriable_request():

All goes well, but one thing: the final results content data from both the partial content of prematurely aborted request and the final and complete data of the second request.
It happens because both dataset are accumulated in the _stream above.

Do you agree it's bug and that on retry the _stream shall be resetted (e.g. seek(0))? Do I miss a use case where it makes sense to accumulated data, or we were just assuming data would be empty anyway on exception?

If I did not miss anything I'll submit a PR to fix it.

Many thanks in advance.
Fred

[andrewsg]

Hi, thanks for your report.

I'm unclear on whether we can trust that _stream will always start a download at the zero position, or for that matter whether it will always be seekable in the first place. This issue hasn't come up for us before because, as you're probably aware given that you're working around it now, by default requests library timeouts only trigger before the first byte is sent, so therefore retries based on timeouts never trigger mid-stream unless they are overridden by a system like yours.

Before we can really verify that this change will be safe we'll need to update our retry conformance test suite internally, so we'll start looking at that soon. This might take a bit of time as we improve support for complex failure cases in our test emulator.