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

reqwest::blocking::ClientBuilder does not have a function tcp_keepalive() #1098

Closed
thegatsbylofiexperience opened this issue Dec 3, 2020 · 7 comments

Comments

@thegatsbylofiexperience
Copy link

HI,

On the current version 0.10.9 I have a windows baset reqwest client connecting is return a weird error, which it was not in v0.10.8. I have setup the client to be reused on multiple requests to the server.

thread '' panicked at 'called Result::unwrap() on an Err value: reqwest::Error { kind: Request, url: Url { scheme: "http", host: Some(Ipv4(127.0.0.1)), port: Some(1234), path: "/some_path", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp set_keepalive error", Os { code: 0, kind: Other, message: "Success." })) }'

I am trying to use the blocking client builder to add the tcp keepalive timeout. However it does not have one available.

From the change log it seems like it was an added feature in 0.10.9.

I am happy to help with a PR if you want me to have a go at adding this to reqwest. Please let me know.

Thanks.

@seanmonstar
Copy link
Owner

It looks like the option wasn't added to the blocking::ClientBuilder. If you want to add it in a PR, that'd be welcome :)

@rneswold
Copy link

I'm having a similar problem on NetBSD 9. I'm using the huebridge crate. When I specify reqwest 0.10.8, I can control the Philips lights. When I upgraded to 0.10.9, I get this:

HueError(Reqwest(reqwest::Error { kind: Request,
                                  url: Url { scheme: "http",
                                             host: Some(Ipv4(192.168.1.4)),
                                             port: None,
                                             path: "/api/.../lights/8/state",
                                             query: None,
                                             fragment: None },
                                    source: hyper::Error(Connect,
                                                         ConnectError("tcp set_keepalive error",
                                                                      Os { code: 42,
                                                                           kind: Other,
                                                                           message: "Protocol option not available" })) }),
                  State { next_error: None,
                           backtrace: InternalBacktrace { backtrace: None } })

@Martichou
Copy link
Contributor

Martichou commented Dec 14, 2020

@rneswold can we see an example of your code ?
Have you tried using HEAD instead of 0.10.9?

@rneswold
Copy link

reqwest is getting used indirectly by huebridge. So I have stuff like:

if let Err(e) = bridge.set_light_state(light, &state) {
    warn!("light {} : error {:?} sending state {:?}", light, e, state)
}

huebridge hasn't changed -- I've been using 0.3.0 for months. Here's where it's defined:

https://github.com/NikoWoot/huebridge/blob/a3be3a2b851df97003e9fb3157a69c2ff08b789f/src/bridge.rs#L118

I haven't tried a non-released version. Is there an easy way to do that with cargo?

@seanmonstar
Copy link
Owner

Sorry about the breakage. The problem comes from reqwest defaulting to setting TCP_KEEPALIVE starting in release 0.10.9, and hyper's HttpConnector fails the connection if an option is unavailable. So, here's my plan:

  • Release a new version of reqwest disabling TCP_KEEPALIVE by default.
  • Get hyper to stop failing if setting an error fails, and instead just log a warning.
  • Once it's fixed in hyper, we can re-enable TCP_KEEPALIVE by default.

@seanmonstar
Copy link
Owner

Reverting the default is in a PR here: #1113

The actual title of this issue is related to a missing method, which was already fixed on master, so I'm going to close here.

@rneswold
Copy link

Sorry about the breakage.

Not a problem. Thanks for being so responsive.

hrkfdn added a commit to hrkfdn/ncspot that referenced this issue Dec 16, 2020
should fix crashes on platforms where tcp_keepalive is not
allowed (e.g. openbsd)

see also:
- seanmonstar/reqwest#1113
- seanmonstar/reqwest#1098
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

No branches or pull requests

4 participants