hjr3 / hyper-timeout

A timeout connector for the hyper client
Other
25 stars 12 forks source link

Failing https requests #13

Closed jaspervdm closed 4 years ago

jaspervdm commented 4 years ago

I've noticed that when we use the HttpsConnector from hyper_rustls in combination with the TimeoutConnector from this crate, request fail with errors such as hyper::Error(ChannelClosed), hyper::Error(IncompleteMessage) or hyper::Error(Parse(Version)).

Here's a toy example:

use hyper_timeout::TimeoutConnector;
use hyper::{Body, Client, Request};
use hyper_rustls::HttpsConnector;
use std::time::Duration;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> { 
    let connector = HttpsConnector::new();
    let mut connector = TimeoutConnector::new(connector);
    connector.set_connect_timeout(Some(Duration::from_secs(20)));
    connector.set_read_timeout(Some(Duration::from_secs(20)));
    connector.set_write_timeout(Some(Duration::from_secs(20)));
    let client = Client::builder().build::<_, Body>(connector);
    let req = Request::builder()
        .method("GET")
        .uri("https://twitter.com/")
        .body(Body::empty())
        .unwrap();

    let res = client.request(req).await.unwrap();
    println!("STATUS = {:?}", res.status());

    Ok(())
}

This one panics on the request unwrap on websites such as twitter and reddit. When I'm using the HttpsConnector directly (comment out lines 9-12), it doesn't panic. This makes me believe the problem is with the timeout connector. Do you have a suggestion what could be going wrong here?

hjr3 commented 4 years ago

I will take a look!

hjr3 commented 4 years ago

Looks like the issue is with the default h2 support in hyper-rustls. If you hit a site that does not support h2 (try https://archive.org), then the request works as expected.

I will chase this down some more.

hjr3 commented 4 years ago

@jaspervdm can you please confirm this fixes the issue? i can release a new version first if that is easier as well.

jaspervdm commented 4 years ago

I just tried it out and I can confirm it fixes the issue! Thanks for the quick fix. Releasing a new version would be ideal for us, if possible.

hjr3 commented 4 years ago

@jaspervdm https://github.com/hjr3/hyper-timeout/releases/tag/0.3.1

jaspervdm commented 4 years ago

Thank you!

gahag commented 3 years ago

@hjr3 I'm having this same issue once again. Are you aware of anything that may have introduced a regression? Thanks for the attention.