Improve error handling (HttpErrorHandler)

[richdougherty]

There has been a bit of discussion about error handling in Play, both online (#6153, #6116) and offline. I'm creating this issue to capture some of these discussions in one place.

From #6153:

• When to include a Result object in the error information?
• How to tell "where" an error happens? Did it happen in application code or in Play code? (My comment: could the stack trace could be used for this?)
• Do we need more types of errors/exceptions? We should probably try and avoid throwing plain RuntimeExceptions, at least.
• Do we need more types of errors/exceptions so it's easier for applications to differentiate between types of exceptions.
• We need a way to provide a status code for server errors (5xx) instead of always defaulting to 500 errors.
• Also need to think about headers, e.g. a 505 usually needs a Connection: close header, a 503 status can be paired with a Retry-After header, a 401 status should include a WWW-Authenticate header.

From #6116:

• Overriding specific error codes (e.g. 404) but not others.
• Dev vs prod mode handling.

Suggestion from @gmethvin:

• Replace the various onXxxError methods with an onError method that takes some sort of HttpError object as an argument. This allows us to evolve the API more easily because we can add fields to the HttpError object over time instead of needing to add new parameters to methods in the HttpErrorHandler interface.

[wsargent]

Also see #2581 and https://groups.google.com/forum/#!topic/play-framework-dev/nE6m9_O0TNw

[mkurz]

We should also pass the current Http.Context to the various onError methods in HttpErrorHandler.

This is not really relevant anymore, because in starting with 2.7 there is not Http.Context anymore (deprecated).
If you want to access information from the original request (e.g. request attributes) you can use a workaround like this: #8303 (comment)
However we might want to think about if it makes sense to give users access the the "original" request in the state it was just before an exception occurecd - if that is even possible to implement somehow (if not, the workaround is good enough I think).

[mkurz]

Adding another point to the list here:
Because now that we have the JsonHttpErrorHandler (#8299), the HtmlOrJsonHttpErrorHandler and the more generic PreferredMediaTypeHttpErrorHandler (#8540) (also see docs), it might make sense for the onClientError method to not just accept a String as message, but somehow also accept a JSON object (or some other type as well). See #10259 (comment)

[mkurz]

• How to tell "where" an error happens? Did it happen in application code or in Play code? (My comment: could the stack trace could be used for this?)

This is a very very good point.

I have/had various use cases where I want(ed) to know where an error comes from. Specially for client errors: Usually client errors (the 4xx range) originate from within actions methods where filters are processed already (like not found / bad request etc.). There is one exception however: When the server backend returns early because the request was too large (413 / REQUEST_ENTITY_TOO_LARGE):

• https://github.com/playframework/playframework/blob/2.8.1/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala#L422
• https://github.com/playframework/playframework/blob/2.8.1/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala#L121

in that case the request will not be processed further, no filters will be called etc. Basically the Play filters/actions code will never be reached (given that netty is not Play).
Now, in an error handler there is no way to distinguish between an client error originating in user code or the mentioned client error triggered in a server backend. However, in an web app, where the user is logged in, for such 4xx errors you can still display a nice error message where the user is still logged in, meaning there is an avatar image, the menu, a "log out" link etc. However now that for the server backend client error, the filters did not run yet, you will get an exception because the CSRF filter did not run as well so a "log out" form rendering the CSRF token will fail.
Things like this happend to me quite often, where I want to run filter logic in errorhandlers, but only if the filters did not run yet. Also think about the security headers (filter) or the CSP tokens (filter)...

Long story short: There are definitely use cases where its good to know the origin of an request passed to an error handler (another use case: #8242 (comment)), therefore I am working on a pull request which adds a HttpErrorInfo request attribute to such requests, containing an origin property.

[mkurz]

Pull request to track origins of errors: #10270