SyliusResourceBundle icon indicating copy to clipboard operation
SyliusResourceBundle copied to clipboard

Should we close connection on RejectedExecutionException?

Open kachayev opened this issue 5 years ago • 2 comments

When rejecting an invalid request, we close the underlying connection here. But when rejecting it because of RejectedExecutionException, we do not. We probably do not need to do this, when reject-handler is defined by the user, but in case we return 503 status code to the client by ourselves.... it might be the case that request body was not consumed and the client misbehaved ignoring our premature response. To make sure this would not break the next request decoding, we probably need to clean everything up by closing the connection. It may lead to increased load on the server when rejected clients will try to reconnect... so I'm wondering about what is the best option here. /cc @ztellman

kachayev avatar Dec 23 '18 16:12 kachayev

We should close the connection for all 4xx and 5xx responses, I have a local change to this effect but it broke some tests and I haven’t tracked down why yet. On Sun, Dec 23, 2018 at 10:27 PM Oleksii Kachaiev [email protected] wrote:

When rejecting an invalid request, we close the underlying connection here https://github.com/ztellman/aleph/blob/be1ee04c130660ce69ee7c6603132b36be79ec1f/src/aleph/http/server.clj#L211-L219. But when rejecting it because of RejectedExecutionException, we do not https://github.com/ztellman/aleph/blob/be1ee04c130660ce69ee7c6603132b36be79ec1f/src/aleph/http/server.clj#L160-L168. We probably do not need to do this, when reject-handler is defined by the user, but in case we return 503 status code to the client by ourselves.... it might be the case that request body was not consumed and the client misbehaved ignoring our premature response. To make sure this would not break the next request decoding, we probably need to clean everything up by closing the connection. It may lead to increased load on the server when rejected clients will try to reconnect... so I'm wondering about what is the best option here. /cc @ztellman https://github.com/ztellman

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ztellman/aleph/issues/457, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB6PzSBxvMtnqXxBhQ2a1afiJT0NQzhks5u77YHgaJpZM4Zf6iI .

ztellman avatar Dec 23 '18 17:12 ztellman

If you share this in a separate branch, I can take a look too. I'm working with connection cycle interruptions right now trying to figure out how to deal with them from the point of user's code (both for the server and the client) and how to keep channel's pipeline stable if anything goes wrong.

kachayev avatar Dec 23 '18 17:12 kachayev