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 Future::timeout() method #6047

Closed
petrzjunior opened this issue Oct 4, 2023 · 5 comments · Fixed by #6276
Closed

Add Future::timeout() method #6047

petrzjunior opened this issue Oct 4, 2023 · 5 comments · Fixed by #6276
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-time Module: tokio/time

Comments

@petrzjunior
Copy link

Is your feature request related to a problem? Please describe.
Currently, the code to timeout a Future is

let _ = timeout(Duration::from_secs(3), listener.accept())
    .await
    .expect("timeout")
    .expect("accept failed");

I propose it should be possible to write

let _ = listener.accept()
    .timeout(Duration::from_secs(3))
    .await
    .expect("timeout")
    .expect("accept failed");

This results in cleaner code following the fluent style of writing.

Describe the solution you'd like
I propose adding a Future extension method

fn timeout(self, timeout: Duration) -> Timeout<Self>

Describe alternatives you've considered
The current method timeout(Duration, Future) works, but is less readable.

Additional context
The proposed method used to be in Tokio in the past:

fn timeout(self, timeout: Duration) -> Timeout<Self>
where
Self: Sized,
{
Timeout::new(self, timeout)
}

It got removed in #1774 refactoring.

Similar method is implemented in async_std since 2019 async-rs/async-std#600.

@petrzjunior petrzjunior added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Oct 4, 2023
@Darksonn Darksonn added the M-time Module: tokio/time label Oct 5, 2023
@Darksonn
Copy link
Contributor

Darksonn commented Oct 5, 2023

We don't currently have any extension traits for futures in Tokio, and I'm somewhat reluctant to add one.

@chris-leach
Copy link

@Darksonn can you expand on your reluctance at all? I agree that the postfix interface is more ergonomic, especially when there are several chained method calls before the await, as opposed to after:

timeout(
    Duration::from_secs(1),
    RequestBuilder::new(/*...*/)
        .prepare(/*...*/)
        .check(/*...*/)
        .build(/*...*/)
        .send(/*...*/),
)
    .await
    .wrap_err(/* ... */)?;

vs

RequestBuilder::new(/*...*/)
    .prepare(/*...*/)
    .check(/*...*/)
    .build(/*...*/)
    .send(/*...*/)
    .timeout(Duration::from_secs(1))
    .await
    .wrap_err(/* ... */)?;

@YuanYuYuan
Copy link

Also curious about why FutureExt was removed from tokio .

@Darksonn
Copy link
Contributor

We removed FutureExt because we wanted to move away from combinators such as map and so on. Async/await syntax has made those mostly obsolete. If we are to add this, I'd prefer to add it to tokio-util.

@evanrittenhouse
Copy link
Contributor

evanrittenhouse commented Jan 7, 2024

Feel free to assign this to me, I'll add said extension trait to tokio-util

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-time Module: tokio/time
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants