Deep strict evaluation of Response

[negatratoron]

Without this change, Happstack's failResponse is not always sent.
For instance:

main :: IO ()
main = do
  let sendResponse = ok $ toResponse (fromJust Nothing :: String)
  simpleHTTP nullConf sendResponse

This server tries to send a string, but can't because the string's
value is bottom. Before this change, the server replies with status
200 and empty content, while outputting "HTTP request failed with:
Maybe.fromJust: Nothing" to stderr. This is because the response is
only evaluated into WHNF inside the exception handler in Handler.hs,
and the exception is caught by the surrounding catch block in
Listen.hs. After this change, the server replies with status 500 and
the failResponse page.

It's possible that this slows down the Happstack server. Some parts
of the Response object may be evaluated but not needed. Strictly
evaluating the response at a finer level might be necessary.

Additionally, with NFData Response, clients can call deepseq
themselves on the response object in order to catch any bottom values
that it contains, log them and display a custom error page:

import Control.DeepSeq (deepseq)
import Control.Monad.Catch (SomeException, handle)

handleServerPartError :: ServerPart Response -> ServerPart Response
handleServerPartError s = handle errorPage $ do
  res <- s
  deepseq res (return res)
  where
    errorPage :: SomeException -> ServerPart Response
    errorPage _ = (internalServerError $ toResponse "Custom error page!")

main :: IO ()
main = do
  let sendResponse = ok $ toResponse (fromJust Nothing :: String)
  simpleHTTP nullConf (handleServerPartError sendResponse)

Let me know if you want to merge in this patch. I'll be using it in
my own applications for sure, unless it proves to cause any
problems. :)

[negatratoron]

Also, if the response is large, the whole thing needs to sit in memory before it can be sent, which would be pretty bad for video streaming sites.

Cale on IRC mentions that if Happstack WERE to deepseq the response, it might as well just use the strict ByteString instead of the lazy one...

A better solution might be to trap errors that are thrown from within putAugmentedResult.

It's very hard to say what the right thing is. Suppose you're streaming a video, and there's a bottom value halfway through it.

I'll probably close this PR and create an issue instead.

[stepcut]

Forcing everything into RAM is definitely a no-go. In fact, we had work to make sure that did not happen by accident.

Forgetting about the specifics of the implementation, I think there is a "can't have your cake and eat it too" problem here.

One the one hand we would like to be able to generate the response body on the fly. Currently we do that via a lazy string -- but it could be done via conduits, pipes, machines, etc. But, since it is happening on the fly, there is always a chance for something to go wrong in the middle of generating the response body.

On the other hand, you seem to want to return a '500 internal server error' response. But, that would require us to go back in time. When sending a response, we first send the response headers with the response code (such as 200 OK), and then we send the response body. So, if an error occurs while generating the response body, it's too late to send 500, because the response code has already been sent.

So, I guess the question is -- what is the correct way to handle this issue in the HTTP system. Does the spec provide some way to say 'whoops, I said this was ok, but now I realize there is a problem'. I think maybe when using chunked responses, there is some mechanism for that. But it has been a long time since I read the spec.

With the way things are currently implement (lazy by default), I believe that you can, in your application, force the response bodies to be strict. So that gives you the most flexibility. In fact, if you look at the file serving stuff we already provide the option to serve files using strict means rather than lazy,

http://hackage.haskell.org/package/happstack-server-7.5.0.1/docs/Happstack-Server-FileServe-BuildingBlocks.html#v:filePathStrict

You could also create a filter that would force response bodies to be strict automatically (though you'd need some sanity check to ensure you don't force a whole video into RAM).

[negatratoron]

I just changed this patch so that it doesn't internally evaluate all Response objects, but still enables them to be deeply evaluated if the application writer wishes to. I changed the commit message too, to match what is actually done.

[codygman]

I'll say that at work these not being strictly evaluated led to us missing application errors once upon a time. If I recall we basically just deepseq the entire Happstack.Server.Response and things seem to work well enough.

I mostly mention it to add a small weight to this issue and to ask about any other downsides that approach may have.