Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve response body when retrying/redirecting #876

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Jul 11, 2022

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.

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.
@quinnj quinnj requested a review from fredrikekre July 11, 2022 13:42
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2022

Codecov Report

Merging #876 (79cca8c) into master (1dc463b) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #876      +/-   ##
==========================================
+ Coverage   79.97%   80.01%   +0.03%     
==========================================
  Files          36       36              
  Lines        2867     2867              
==========================================
+ Hits         2293     2294       +1     
+ Misses        574      573       -1     
Impacted Files Coverage Δ
src/clientlayers/StreamRequest.jl 97.14% <100.00%> (+1.42%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@quinnj quinnj merged commit 98a0b69 into master Jul 11, 2022
@quinnj quinnj deleted the jq/preserve_response_body branch July 11, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants