search

jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.86k stars 1.91k forks source link

Improve handling of idle timeouts when the handling itself is idle #12301

Closed gregw closed 1 month ago

gregw commented 1 month ago

Currently if a handler is idle (or blocked) when an idle timeout occurs, it is essentially a noop, unless the handler has an idle timeout listener, or subsequently reads from the request.
This change ensures that all subsequent writes will also fail.

sbordet commented 1 month ago

This change ensures that all subsequent writes will also fail.

I don't think this would work, because we need to be able to write a response if an idle timeout or a failure happens.

What is the reason behind this change?

sbordet commented 1 month ago

@gregw so with the removal of the parameter alwaysFailFutureWrites (always false), then writes are not failed.

I don't think failing writes is correct, because we want to be able to write an error response in case of failures or idle timeouts.

gregw commented 1 month ago

@sbordet

@gregw so with the removal of the parameter alwaysFailFutureWrites (always false), then writes are not failed.

I don't think failing writes is correct, because we want to be able to write an error response in case of failures or idle timeouts.

I'm not understanding your comment? This PR is no longer failing writes after a failure. It did for a while, but after discussions we couldn't convince ourselves that it was right to do so. I'm not entirely convinced it is wrong to do so, but keeping the behaviour the same as currently except ensuring the Runnable is executed is a good step.

Why is it so important to write a response when there is a failure? Most times when this happens, the response will never make it to the client, as the connection will have failed in some way, hence the failure/timeout. I know we have always strived to do this, but I'm wondering what is the value? The cost is that we can sometimes ignore real errors and not deliver them to an application, which then doesn't see those failures on subsequent writes. Perhaps this PR to improve the invocation of error listeners will avoid the problem, but I'm still not 100% sure.