Do not write 500 if an exception is already handled
Closed this issue · 4 comments
Today, Bandit sends a 500 response when an exception is rescued. However, when using a plug such as Plug.ErrorHandler, where an exception is handled and then re-raised, Bandit ends up sending a response after a response has already been written by the error handling function.
Perhaps this isn't too problematic, but I have found that some HTTP clients, such as Go's, will complain about an unexpected response being written to the open socket, with an error such as:
2024/01/11 11:59:51 Unsolicited response received on idle HTTP channel starting with HTTP/1.1 500 Internal Server Error
date: Thu, 11 Jan 2024 16:59:51 GMT
content-length: 0
vary: accept-encoding
I couldn't track down where the exception handling happens in Plug.Cowboy, but note that when using it, this issue doesn't appear to occur.
Yep, we should be doing a better job here. The relevant fix will be to check the process mailbox to see if we've sent ourselves a {:plug_conn, :sent} message, and to quell sending if so.
The scope of this is only HTTP/1. HTTP/2 does the right thing already since we use a multi-process model by necessity, and error trapping & reporting is an intrinsic part of the HTTP/2 model.
Cowboy gets a pass on this because their process model lets them keep state about whether a response has already been sent in the connection process, separate from the handler process. Their HTTP/1 process model is less efficient, but more robust to cases like this.
I'll get a fix up for this in the next few days. Thanks for the report!
Fix published in 1.1.3