Implement request timeout by adding deadline to base stream #39
Conversation
|
Could you add a test for this too? I think you could use an end-to-end test in the tests directory. Create a TcpListener that accepts streams and writes a partial response only to the stream, i.e. by not terminating the request properly, then make an http request to that listener and verify if it timeouts. I've been thinking about it however, and I thought of a case which might break because of this. Say you download a really large file that takes a long time to transfer. If the request timeout is short it would most probably be reached every time. That use case would probably be best served by a connect timeout and a read timeout. It might just be best to allow people to configure a connect timeout and a read timeout. The happy code already supports a timeout. |
Personally, I am not a fan of this approach here at all due what I said before: I think this is pushing attohttpc into use cases for which it was not designed. Exposing the underlying connect and read timeouts only seems more reasonable to me. |
|
Yeah I think I agree. We should only go for a connect & read timeout. We should also set default values of like 30s for each, so that we don't hang programs. If the timeouts are too short for some use cases the defaults can just be bumped via the RequestBuilder. |
|
I'll extend #37 to support connect timeout, add the defaults and end-to-end tests. (Consider the 30s value, the current connect timeout in |
|
I think we can move the default timeout value away from the happy module and also set it to 30s. I think 30s is a little bit more standard than 10s. |
|
Closing in favor of #37 |
No description provided.