-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpClient] Fix content swallowed by AsyncClient initializer #39228
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
Conversation
d2fb04a
to
f5c0c01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @jderusse for your work on this.
As discussed on Slack, this should be fixed in |
785fbe8
to
bfc4b10
Compare
bfc4b10
to
76c9991
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this and I can confirm that it fixes the issue
1df90ba
to
b454235
Compare
b454235
to
d324271
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after some changes.
Thank you @jderusse. |
I'm not sure if it should be fixed in RetryableHttpClient or AsycClient.
The issue is: when the Strategy needs the body to take a decision BUT decide to NOT retry the request, the content is "lost"
In fact, when the first chunk is yield, the AsyncResponse's initializer is stopped, and nothing consume the remaining chunks. Moreover, because the
passthru
were disabled before yielding the first chunk in RetryableHttpClient, the callback is never called again to yield the remaining content.