Skip to content

Add DeadlineBody for non-resetting body timeouts#688

Open
jlizen wants to merge 4 commits into
tower-rs:mainfrom
jlizen:feat/absolute-timeout-body
Open

Add DeadlineBody for non-resetting body timeouts#688
jlizen wants to merge 4 commits into
tower-rs:mainfrom
jlizen:feat/absolute-timeout-body

Conversation

@jlizen
Copy link
Copy Markdown
Member

@jlizen jlizen commented May 22, 2026

Closes #623

Summary

Adds AbsoluteTimeoutBody, a body wrapper that enforces a hard deadline on the entire body transfer. Unlike TimeoutBody (which resets its timer each time a frame arrives), AbsoluteTimeoutBody starts a single Sleep at construction and never resets it. If the body isn't fully consumed before the deadline, poll_frame returns a TimeoutError.

This is useful for public endpoints where you want to cap total time spent on a request regardless of how frequently data trickles in. A slow client sending one byte per second would never trip TimeoutBody's idle timeout, but will correctly trip AbsoluteTimeoutBody.

I went with a new standalone type rather than a config flag on TimeoutBody. This keeps the existing API unchanged and avoids branching in TimeoutBody::poll_frame. The corresponding RequestBodyAbsoluteTimeout and ResponseBodyAbsoluteTimeout service/layer types mirror the existing idle-timeout equivalents.

It's a bit verbose, but I think it's ok. Name bikeshedding is invited by reviewers.

Shares the existing TimeoutError type since the failure mode is the same: body data didn't arrive in time.

Testing

Covers the two scenarios that distinguish this from TimeoutBody:

  • Absolute timeout fires even when frames arrive steadily but total transfer exceeds the deadline
  • Body completes successfully when all frames arrive within the absolute deadline

Also includes basic single-frame happy/error path tests, and verifies the error downcasts to TimeoutError.

@seanmonstar
Copy link
Copy Markdown
Collaborator

This is just my rambly opinion: I wouldn't expect a body to have a full timeout, which is shared over multiple chunks. Instead, I would expect to set a timeout over some operation consuming the body. Like, if I'm collecting the whole body into a flat Vec or something, I would expect to timeout that whole future.

It kind of confuses me that the timeout on this body would continue to count even if I wasn't actively polling the body. Like if I polled some of the body, and then started doing something else, and then came back, and it triggers the timeout.

And yes, I know there's just such a timeout in reqwest, and I find that equally weird.

@jlizen
Copy link
Copy Markdown
Member Author

jlizen commented May 23, 2026

@seanmonstar - Yeah, fair, agreed the semantics are a bit unusual if you think of it as a "body timeout" in isolation.

The use case I had in mind (and the one in #623) is less "timeout on reading the body" and more "hard cap on how long this request can hold resources." For a public-facing service, you want to bound the total wall-clock time a single connection can occupy a task slot, memory for buffering, etc. A slow client sending one byte per second will never trip an idle timeout, but you still want to evict it eventually. The original issue frames it the same way: "ensure that users don't take up too much of your server's time."

You're right that if the consumer stops polling mid-body to go do other work, the clock keeps ticking, which could be surprising. In practice though, the expected pattern is a tight drain loop (body.collect().await or equivalent), so the timer is effectively measuring client transfer speed plus however much processing overhead.

The alternative is wrapping the consume operation in tokio::time::timeout(), which works fine for application code but is hard to apply uniformly as middleware. You'd need each handler to remember to do it. This body wrapper lets you express "no request body transfer may exceed N seconds" as a layer, which is the same niche that reqwest::Client::timeout() fills on the client side.

I think there's a third variant that would address the "only count time while actively polling" concern: a cumulative-poll-duration timeout that tracks elapsed time only while a poll is pending and pauses when the consumer isn't polling. That would be useful for cases where you want to enforce minimum client throughput without penalizing server-side processing gaps. It's a meaningfully different semantic though, and probably worth exploring as a separate issue/PR if there's interest. I've actually helped people implement that inside my company. That's a great idea to have and I'd be glad to open an issue for it (and probably we can upstream some of the internal work we've done). This PR targets the simpler wall-clock-deadline case from #623.

Happy to add a doc note calling out that the deadline is wall-clock from construction (not cumulative poll time), so the behavior is clear to users. Also open to naming suggestions if AbsoluteTimeoutBody doesn't convey the right mental model. DeadlineBody is another option that might signal "hard wall-clock cap" more directly.

Comment thread tower-http/src/timeout/absolute_body.rs Outdated
/// # Ok(())
/// # }
/// ```
pub struct AbsoluteTimeoutBody<B> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, wouldn't it make more sense for this to live in http-body-util?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I'd be open to contributing the different types of timeout bodies to that crate, and using them internally in tower-http (since we still do want the service/layer composition helpers).

But, I don't maintain that crate currently and didn't want to add maintainer load, and anyway the TimeoutBody currently lives here.

If you have perms on http-body-util and want to review such a PR, glad to cut one and update this one accordingly :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'm not a maintainer on any of the hyperium crates.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question to you @seanmonstar

@rcoh
Copy link
Copy Markdown

rcoh commented May 26, 2026

personally I'd weigh in that this is generally a useful abstraction although one that is often going to have surprising results. It will only be useful for a very narrow range of specific use cases (which isn't inherently on a problem).

We have a more sophisticated version of this in the Rust SDK that is basically this, but the timer resets when you get more data (so it is a minimum-speed-limit) basically. This seemed good in practice for being able to to a good job of terminating slow connections but it seems like it mostly causes issues.

I would consider naming this DeadlineBody, a body that must be completed by some deadline. I think this may also help with @seanmonstar's comments since it becomes very clear when this body is going to be cancelled (at the deadline)

@jlizen
Copy link
Copy Markdown
Member Author

jlizen commented May 26, 2026

We have a more sophisticated version of this in the Rust SDK that is basically this, but the timer resets when you get more data (so it is a minimum-speed-limit) basically. This seemed good in practice for being able to to a good job of terminating slow connections but it seems like it mostly causes issues.

I don't mind adding in an API knob that has this behavior, seems useful enough, though I don't think it's the right default. Or do you think it is mostly a footgun @rcoh and not worth including?

ACK on it being a fairly narrow range of use cases, I can improve docs to be clearer about relevance.

DeadlineBody

++ i'll change it

@jlizen jlizen changed the title Add AbsoluteTimeoutBody for non-resetting body timeouts Add DeadlineBody for non-resetting body timeouts May 27, 2026
@jlizen
Copy link
Copy Markdown
Member Author

jlizen commented May 27, 2026

@rcoh updated to use DeadlineBody naming and improved docs around when to use it / to be very explicit that wall clock time keeps ticking.

Once this lands, I'll cut an issue to http-body-util to see if we can move both body variants (deadline and timeout) over there, at which point we can cut over the internal usage here.

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.

Suggestions: TimeoutBody with absolute timeout (no reset on received frame)

4 participants