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

Add BodyTimeout middleware #295

Closed
seanmonstar opened this issue Sep 28, 2022 · 3 comments · Fixed by #303
Closed

Add BodyTimeout middleware #295

seanmonstar opened this issue Sep 28, 2022 · 3 comments · Fixed by #303
Labels
A-new-middleware Area: new middleware proposals C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Experience needed to fix: Easy / not much

Comments

@seanmonstar
Copy link
Collaborator

Feature Request

Motivation

Besides the larger service timeouts, it's also common for people to want to add a timeout to the reading of chunks from a body, either for the request as a server, or for the response of a client.

Proposal

  • Make the timeout module a folder, with service and body timeout files.
  • The body timeout should have a struct TimeoutBody<B>.
    • Implement Body for TimeoutBody<B> where B: Body, such that polling for data will check the inner body poll, and also a timeout.
      • The timeout should be reset if a chunk can be returned.
  • RequestBodyTimeout and ResponseBodyTimeout layers can be added (Service and Layer) which essentially do what MapRequest and MapResponse do, wrapping the Request<B> into Request<TimeoutBody<B>> (and same with response).
@seanmonstar seanmonstar added A-new-middleware Area: new middleware proposals C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Experience needed to fix: Easy / not much labels Sep 28, 2022
@seanmonstar
Copy link
Collaborator Author

@82marbag here's what we discussed. We can add more details here if needed.

@Velfi
Copy link

Velfi commented Oct 6, 2022

It would be nice if this middleware could also handle streams that respond but only send a drip feed of data

@Velfi
Copy link

Velfi commented Oct 6, 2022

I made a draft PR a while back that looked into this a bit, linking here in case it's useful
smithy-lang/smithy-rs#1627

82marbag pushed a commit to 82marbag/tower-http that referenced this issue Oct 11, 2022
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>
LucioFranco pushed a commit that referenced this issue Oct 31, 2022
## 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 and `poll` on sleep the time runs out and the sleep is done, a timeout error is immediately returned

Co-authored-by: Daniele Ahmed <ahmeddan@amazon.de>
Co-authored-by: Harry Barber <hlbarber@amazon.com>
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Oct 20, 2023
This middleware acts roughly similar to http://nginx.org/en/docs/http/ngx_http_core_module.html#client_body_timeout and ensures that there is a timeout applied when reading request bodies.

see tower-rs/tower-http#295
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Oct 20, 2023
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Oct 20, 2023
This middleware acts roughly similar to http://nginx.org/en/docs/http/ngx_http_core_module.html#client_body_timeout and ensures that there is a timeout applied when reading request bodies.

see tower-rs/tower-http#295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-new-middleware Area: new middleware proposals C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants