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

fix: propagate body stream error to close function (#1743) #1757

Merged

Conversation

mdenushev
Copy link
Contributor

@mdenushev mdenushev commented Apr 12, 2024

When using body streaming with keep-alive connections current version ignores write errors and returns broken connection to pool. Fix propagates write errors to bodyStream close function and closes connection if error occurs.

Fix #1743

@mdenushev mdenushev marked this pull request as draft April 12, 2024 14:29
@erikdubbelboer
Copy link
Collaborator

The fix looks good. I'm guessing you are currently trying this in production? When you think it's ready I can merge it.

@mdenushev
Copy link
Contributor Author

I tried fix yesterday on production, but error occurs (less frequent but still), so I'm still looking for another place causing it

@mdenushev
Copy link
Contributor Author

I found reason causing bug after applied fix, upstream service sends Continuation packet after request, so HostClient reads 0\r\n\r\n correctly, this pull request is finished.

@mdenushev mdenushev marked this pull request as ready for review April 17, 2024 17:15
@erikdubbelboer erikdubbelboer merged commit 57b9352 into valyala:master Apr 22, 2024
18 of 19 checks passed
@erikdubbelboer
Copy link
Collaborator

Thanks!

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.

Timeout stream response connection does not clear buffer data for re-use
2 participants