HTTPClient keep-alive is not initialized properly #756
Conversation
|
Looks good. I'd also add a check if the connection is actually established, so that the possibly bogus log message is also suppressed. BTW, shouldn't |
|
This is currently needed for the HTTPClient to work correctly, according to my tests |
| } | ||
|
|
||
| @property int maxConnections() const { | ||
| return m_maxConnections; |
There was a problem hiding this comment.
Shouldn't this read maxRequests?
|
Sorry for leaving this open for so long. It's quite a tough review job on this one, but I think everything looks reasonable. One peripheral improvement would be to put the example into a documented unit test (in a nested |
|
That ICE in the Travis output looks ugly. Do you get than when running |
|
Woa, no I tested it before committing and it was perfectly fine. Maybe it's a problem with rebase, I'll try and commit again |
Duration must be at the left of SysTime in an addition Refactored and fixed logical errors Fixed major issues and refactored some more Check if connected before checking if keep-alive limit is expired fix bad server_timeout / ignores keep-alive A keep-alive of 0 means no keep-alive Add better documentation for HTTPClientSettings, unittests not possible without a stable proxy online Fix summary for settings Changed test to unittest Change variable name for max keep-alive requests
|
Looks like an issue with 2.065, I commented out the unittest though b/c it doesn't seem like it could be my fault. |
|
Good to know that it's the unit test. Thanks for testing again. I'll merge now and see if something like a |
HTTPClient keep-alive is not initialized properly
|
Strange, I'm not getting that error even with the test commented in. I'll commit with the unit test again to see where exactly Travis chokes. |
… build goes wrong. See vibe-d#756.
This fixes an issue where the disconnect operation was immediately triggered during the connect routine.