jetty.project
jetty.project copied to clipboard
jetty
Fixes InputStreamResponseListener should have a read timeout #7259
Added constructor with read timeout parameter. Updated implementation to honor the read timeout. Added test case.
Signed-off-by: Simone Bordet [email protected]
This looks redundant with the idle timeout mechanism to me. What's this bringing that idleTimeout cannot do, or what am I missing?
@lorban the idle timeout of the ClientConnector cannot be accessed from this class.
I can use that of the request by default, but only if it has been set by the application.
@sbordet my point isn't where to get the timeout value from, but rather than the idleTimeout mechanism should be the only one that unblocks the awaiting thread after a certain time elapsed, at least IMHO.
Adding an extra timeout looks to me like replicating an existing feature just in case that said feature doesn't work as expected.
@sbordet I'm with @lorban on this? Why is there a second timer? If I understand correctly there is a HttpClient request, that will have one idle timeout, then a listener that can have another. So without this PR, a user can make a HttpClient request and start reading from the input stream listener. They will block waiting for content and if the HttpClient request timesout, then their input stream will fail accordingly. With this PR, the input stream may timeout before the HttpClient has? This makes no sense to me?
Without the second timeout, is there ever a case this listener will wait forever for the HttpClient to succeed or fail? The issue only says:
his is sub-optimal because there is a risk that the caller thread waits forever
But surely that would be a bug in HttpClient?
