http-rs / surf

Fast and friendly HTTP client framework for async Rust
https://docs.rs/surf
Apache License 2.0
1.45k stars 119 forks source link

Please add support for setting request timeout #274

Closed stevepryde closed 2 years ago

stevepryde commented 3 years ago

I'm aware of the workaround in #267 but would really like a more ergonomic solution to set the request timeout in surf if possible.

That workaround requires juggling both the http-client and isahc version to make sure the types line up between them and it feels really brittle. For example http-client v6.2 depends on isahc v0.9, whereas the latest version of isahc is currently 1.0. I wanted to use 1.0 but cannot because of the incompatibility.

Please add the ability to set the request timeout when creating a client, as well as the ability to set/override the request timeout per-request if desired. Thanks.

Fishrock123 commented 3 years ago

This needs work in http-client, which currently does not have any support for options. See https://github.com/http-rs/http-client/issues/61

Shnatsel commented 3 years ago

I believe it's currently possible to set a timeout for the entire request via future::timeout. That's great! However, the following timeouts are still missing:

  1. Connect timeout (the server has to reply for the first time within a certain timeframe)
  2. Read timeout (connection to be dropped after a certain period of inactivity)

The defaults for these values are rather high and it's often needed to override them. For example, the default read timeout is 5 minutes (actually enforced on the TCP level).

Fishrock123 commented 3 years ago

WIP draft PR up for the API in http-client: https://github.com/http-rs/http-client/pull/84

Fishrock123 commented 3 years ago

Also, please be aware this is possible externally with async_std::io::timeout()! (Which is more appropriate than future::timeout.)

fiag commented 3 years ago

to customize isahc::HtttpClient which has a timeout option.

    let client = isahc::HttpClient::builder()
        .tcp_keepalive(Duration::from_secs_f32(3600.0))
        .tcp_nodelay()
        .timeout(Duration::from_secs_f32(10.0)
        .build()?;
    let client = http_client::isahc::IsahcClient::from_client(client);
    let client = surf::Client::with_http_client(client);
    client.get(....)
}
stevepryde commented 3 years ago

to customize isahc::HtttpClient which has a timeout option.

    let client = isahc::HttpClient::builder()
        .tcp_keepalive(Duration::from_secs_f32(3600.0))
        .tcp_nodelay()
        .timeout(Duration::from_secs_f32(10.0)
        .build()?;
    let client = http_client::isahc::IsahcClient::from_client(client);
    let client = surf::Client::with_http_client(client);
    client.get(....)
}

I can confirm this approach works. This issue can be closed.

Shnatsel commented 3 years ago

This requires backend-specific code. Are there any plans to provide backend-independent APIs?

Any client using surf and supporting multiple backends would have to hand-roll this code anyway, so it feels like a much better idea to implement it in surf or http-client once and for all.

Fishrock123 commented 3 years ago

I am actively working on bringing a config like HttpClient's new (still "unstable" flagged) Config to Surf.

Fishrock123 commented 2 years ago

Available in https://github.com/http-rs/surf/releases/tag/v2.3.0