-
Notifications
You must be signed in to change notification settings - Fork 161
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
Timeout on body #303
Timeout on body #303
Conversation
Closes tower-rs#295 Add a timeout on inactive bodies, both on request and response bodies. Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> Co-authored-by: Harry Barber <hlbarber@amazon.com>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
|
||
pin_project! { | ||
/// Wrapper around a [`http_body::Body`] to time out if data is not ready within the specified duration. | ||
pub struct TimeoutBody<B> { |
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.
Taking a big step back, this could live in http_body
crate if there was a standard interface for sleep futures.
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.
Yeah, I think we do want to add a timeout body in http-body-utils, that said we can start with that code in here. I think this is the path I am going with for the retry stuff as well.
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
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.
This is a great start! Left a few comments/questions, lmk if you want to discuss any of it offline.
|
||
pin_project! { | ||
/// Wrapper around a [`http_body::Body`] to time out if data is not ready within the specified duration. | ||
pub struct TimeoutBody<B> { |
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.
Yeah, I think we do want to add a timeout body in http-body-utils, that said we can start with that code in here. I think this is the path I am going with for the retry stuff as well.
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
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 just one last thing and we merge
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
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.
SHIP IT
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Motivation
Explained in #295. This PR closes #295.
Solution
Wrap the body in a
TimeoutBody
.TimeoutBody
will poll a sleep future to check whether the body is inactive and register itself to be awoken. The sleep future is polled and checked right after creation to avoid a potential delay in execution making the executor to never poll the sleep future again. That is, if between creation andpoll
on sleep the time runs out and the sleep is done, a timeout error is immediately returned