Make HTTP/1.1 implementation respect read_timeout configuration

[mtrudel]

The HTTP/1.1 stack doesn't currently respect the top-level read_timeout parameter. This comes down mostly to passing it into Socket.recv calls and returning it on keepalive loops. For testing prior art, see https://github.com/mtrudel/bandit/blob/main/test/bandit/http2/protocol_test.exs#L62

[afsharalex]

Hi, would this be a good first issue to tackle (if no one else is already)?

[mtrudel]

Hi! Thanks for the offer of help!

I've been thinking that this timeout work (as well as the existing HTTP2 timeout work referenced above) belong better as a configurable parameter in the underlying socket as implemented in Thousand Island. Having to carry a timeout value around in Bandit's state seems the wrong pace to do it when the ThousandIsland.Socket island struct is already carrying the underlying socket around (and is, by necessity, the API through which ALL requests to the underlying socket pass so it's a natural place to apply such a timeout).

If you're game for taking that work on it would be much appreciated. I've written up my initial thoughts on the matter at https://github.com/mtrudel/thousand_island/issues/15

Once that work lands, the scope of this ticket can change to actually remove timeout code from the HTTP/2 side, since we'll be able to handle both cases intrinsically using the new Thousand Island timeout feature.

WDYT?

[afsharalex]

Awesome, sounds good!

I'd love to help out with Thousand Island as well :)

Thanks!

[mtrudel]

This landed back in 46622b7be872f3f9ebb8ccc522bbcd6e1f2f990b and I neglected to close this issue