yesodweb / wai

Haskell Web Application Interface
MIT License
834 stars 263 forks source link

Exception response handler doesn't catch exceptions #981

Open omcnoe opened 7 months ago

omcnoe commented 7 months ago

Per docs on Exception response handler, Warp handles application exceptions and returns an HTTP 500 response. This often doesn't actually work due to lazy evaluation of the response body.

import Control.DeepSeq (force)
import Control.Exception (evaluate)
application _ respond = do
  let x :: ByteString = head []
  -- strict evaluation required for Warp to catch the exception:
  -- _ <- evaluate $ force x
  respond $ responseLBS status200 [] x

main = run 8080 application

Instead, connection is immediately terminated with no response to client. What are the consequences of forcing strict evaluation of the body/status/headers here? These are all soon to be evaluated for return to client regardless.

ghc-9.6.4.exe warp-3.3.31

Vlix commented 6 months ago

This defeats the purpose of streaming responses, so at least not all responses should do this.

The question here would be, is warp responsible for this behaviour, or should the user code make sure there are no "suspended" exceptions hanging around? 🤔 I'm not sure either way.

kazu-yamamoto commented 6 months ago

I recommend to specify Strict and StrictData pragmas in the Haskell network programming. Most packages which I maintain have these pragmas.

Vlix commented 6 months ago

@kazu-yamamoto I think @omcnoe is referring to user code in the application. And using -XStrict in a large application might be a lot of overkill.

And that also doesn't do any forcing to NF of values you get from external libraries, so that will probably not even fix this entirely. :thinking:

kazu-yamamoto commented 6 months ago

I know many Haskellers think it's overkill. That's OK. It's up to them whether or not they use Strict and StrictData. But I have suffered from a lot of bugs due to lazy evaluation in Haskell network programming. To specify Strict and StrictData, I needed to change my programs. But I'm not suffering from such bugs anymore.

omcnoe commented 6 months ago

I guess issue here is HTTP doesn't have any kind of "reset response" support in the protocol. HTTP expects that by the time you are sending the first response bytes you know definitively that sending the remainder of the response will not produce any error. But with streaming responses + lazy evaluation that is no longer the case...