On Thu, 16 Nov 2023 13:41:48 GMT, Conor Cleary <[email protected]> wrote:
>> **Problem** >> When using HTTP/1.1 with HttpClient, it was observed that requests >> configured with timeouts at build time fail with a HttpTimeoutException when >> they are redirected to a separate URI by a server (status code 3xx on first >> response). What should happen is that the second request response (so after >> receiving a 3xx code) clears restarts the timer intially set. However, when >> `responseTimerEvent` is registered for the first time, it is not >> unregistered and cleared before starting a second timer. >> >> **Solution** >> This fix addresses the issue by calling `cancelTimer()` in >> `MultiExchange.java` whenever the `newRequest` reference is set to a >> non-null value after calling `responseFilters(response)` on the initial >> response received. This occurs in the case where a status code 3xx is >> received in the initial response. When `cancelTimer()` is called, it now >> unreferences `responseTimerEvent` after cancellation operations take place. >> >> While the fix for the issue was relatively straight forward, the regression >> test is less so. I would point to to the comment located in >> `RedirectTimeoutTest:L119` for an explanation of the testing method. > > Conor Cleary has updated the pull request incrementally with one additional > commit since the last revision: > > Request timeout values increased test/jdk/java/net/httpclient/RedirectTimeoutTest.java line 123: > 121: to each request, 4 iterations will take a guaranteed > minimum time of 2000ms which will ensure that any > 122: uncancelled/uncleared timers will fire within the test > window. > 123: */ Comment explaining testing strategy ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16689#discussion_r1395719114
