Skip to content

Commit

Permalink
Preserve response body when retrying/redirecting
Browse files Browse the repository at this point in the history
This is another iteration in our redirect/retry response body handling.
Previously, when the response body is streamed, we ensured only the
"final" response body is the one written to the user-provided stream.
The only issue is that sometimes it can be helpful to have access to
the intermediate response bodies that ended up being retried/redirected.
This PR refactors the `readbody` machinery a bit to ensure we always read
the response body, and in the case of a user-provided response stream,
we read the response body as a string and store it in the request context.

One motivation I'm working towards with this is providing more nuanced control
over the retry logic, even allowing users to pass in their own `check` function
to determine if a request should be retried or not based on the failed response.
In that scenario, you'll want access to the failed response body, so this will
enabled that.
  • Loading branch information
quinnj committed Jul 11, 2022
1 parent 1dc463b commit 79cca8c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 17 deletions.
34 changes: 17 additions & 17 deletions src/clientlayers/StreamRequest.jl
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,7 @@ writechunk(stream, body::IO) = writebodystream(stream, body)
writechunk(stream, body::Union{Dict, NamedTuple}) = writebodystream(stream, body)
writechunk(stream, body) = write(stream, body)

function readbody(stream::Stream, res::Response, decompress)
# Bail early if we are not going to read anything.
# If the request/response pair are going to be redirected or retried,
# we want to avoid "contaminating" our response body stream.
willread = isbytes(res.body) || (!isredirect(stream) && !retryable(stream))
willread || return

function readbody(stream::Stream, res::Response, decompress::Bool)
if decompress && header(res, "Content-Encoding") == "gzip"
# Plug in a buffer stream in between so that we can (i) read the http stream in
# chunks instead of byte-by-byte and (ii) make sure to stop reading the http stream
Expand All @@ -127,18 +121,24 @@ function readbody(stream::Stream, res::Response, decompress)
close(gzstream)
end
end
if isbytes(res.body)
res.body = read(buf)
else
write(res.body, buf)
end
readbody!(stream, res, buf)
wait(tsk)
else
if isbytes(res.body)
res.body = read(stream)
else
write(res.body, stream)
end
readbody!(stream, res, stream)
end
end

function readbody!(stream::Stream, res::Response, buf_or_stream)
if isbytes(res.body)
# normal response body path: read as Vector{UInt8} and store
res.body = read(buf_or_stream)
elseif isredirect(stream) || retryable(stream)
# if response body is a stream, but we're redirecting or
# retrying, store this "temporary" body in the request context
res.request.context[:response_body] = read(buf_or_stream)
else
# normal streaming response body path: write response body out directly
write(res.body, buf_or_stream)
end
end

Expand Down
7 changes: 7 additions & 0 deletions test/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,13 @@ end
seekstart(req_body)
resp = HTTP.get("http://localhost:8080/retry"; body=req_body, response_stream=res_body, retry=false, status_exception=false)
@test String(take!(res_body)) == "500 unexpected error"
# when retrying, we can still get access to the most recent failed response body in the response's request context
shouldfail[] = true
seekstart(req_body)
resp = HTTP.get("http://localhost:8080/retry"; body=req_body, response_stream=res_body)
@test resp.status == 200
@test String(take!(res_body)) == "hey there sailor"
@test String(resp.request.context[:response_body]) == "500 unexpected error"
finally
if server !== nothing
close(server)
Expand Down

0 comments on commit 79cca8c

Please sign in to comment.