Client timeout

[gsquire]

This is a rough attempt at adding timeout to a Client. For ergonomics, it uses the timeout for both read and write portions of a hyper::Client.

Closes #36

[seanmonstar]

I like that it's a single timeout, for now. What do you think of having a mutator method instead of a constructor? So people would write:

let mut client = Client::new()?;
client.timeout(Duration::from_secs(30));
// or
client.set_timeout(dur);

[gsquire]

I had originally wanted to do something just like that but ran into issues with borrowing.

e.g. Inside the Client implementation:

pub fn timeout(&mut self, time: Duration) {
    self.inner.hyper.set_read_timeout(Some(time));
}

This approach yields the error:

error: cannot borrow immutable field as mutable
  --> src/client.rs:72:9
   |
72 |         self.inner.hyper.set_read_timeout(Some(time));
   |         ^^^^^^^^^^^^^^^^

error: aborting due to previous error

Unless I have a fundamental misunderstanding of borrowing, this seems like it can't be done. Feel free to prove me wrong.

[seanmonstar]

Oh right, the client is behind an Arc. We could change ClientRef to put the hyper::Client inside a Mutex, which would allow us to update it then.

[gsquire]

Ok I can do that then, just like the RedirectPolicy.

[seanmonstar]

I'm sorry I didn't notice the changes earlier. Looks like one change would be needed, and we're good to go!

[seanmonstar]

So Mutexes are tricky. This code will surprisingly hold on to the locked client while the request is sent over the network, and the response headers have been received. I'd forgotten that a mutex would play badly here.

This is one of the very few times when a RwLock is probably a good idea. The reason is that we have to lock it somehow for when we mutate the Client, in the timeout() method. But besides that, we actually don't want exclusive access, and only require read (&client) access. Making use a RwLock means that making a change to the timeout has to wait until any current requests are done, but the requests don't block each other.

[gsquire]

That's a good point. I updated it to use a RwLock.

[seanmonstar]

Thanks!