HTTPClient: Fix condition after timeout where client may remain queued or in-progress #31
Conversation
…d or in-progress This adds unit tests which originally failed due to the client remaining in-progress or in queue. The solution is to add a failure listener which will attempt to remove from queue or in-progress if the client is still active or queued.
|
@jentfoo I Dont really get the condition here. I mean obviously the test completely stops any queue processing so it will break things. But what I dont get is how you would end up in that situation in the first place. I totally get that the processQueue could be super busy and something would end up in the queue for a while, while other things process, but it should be removed once we get to it in the queue. If the queue/stuck/blocked, or for some reason not processing. at all I dont see how adding a side channel for removing things from it would help at all since nothing will ever process (all calls will time out) if the thread is stuck. Do you have a stack trace from this happening? My guess would be this is on a SingleThread processor or maybe the NoThreadProcessor and something is keeping it from processing? |
This closes the connection due to it being in a potentially unknown / unhealthy condition. In doing this we can make the `finished` code go back to closer to what it used to be. However the `hasError` now gets the additional cleanup logic so we can defer to that on any failures coming from the timed out future
|
Iterated on this together in DM's with @lwahlmeier We believe that there is a better solution here, however this is probably the simplest solution, and what we want to move forward with for now |
This adds unit tests which originally failed due to the client remaining in-progress or in queue.
The solution is to add a failure listener which will attempt to remove from queue or in-progress if the client is still active or queued.
@lwahlmeier Do you see any better way to do this, or was there any other mechanism to fix this? I considered trying to make sure we either regularly process over the queue / in-progress looking for failed requests to remove. That may avoid the overhead of the failure listener per-request, but I was unsure on the best way to ensure that it was being regularly inspected.