Refactor exception handling, rendering, and reporting

[tobyzerner]

From #1633 (review):

What I would like to see in beta.9:

• One middleware (always enabled) that converts all known exception types with custom views to pretty error pages, always.
• Another middleware (either Whoops or pretty pages) that a) displays the exceptions and b) logs them. (The logging could even be a separate middleware that re-throws the exceptions, unless that messes with the stack traces.)

IMO, there is no need for NotFound and similar exceptions to ever be handled with the fallback error handling (Whoops or pretty pages) - we are just using the exception mechanism to handle known error cases.

May be worth taking some inspiration from https://laravel.com/docs/5.7/errors

I would also like to see the ability to configure logging - we may as well make use of the power of Laravel's logging component.

---

Step-by-step TODO:

• Return proper status codes for Whoops / debug mode
• Token error message - notes from PR (Change token error message #1528)
• Pretty error pages should differ based on exception type, not status code (e.g. CSRF token mismatch)
• Exceptions should expose whether they are loggable / reportable (see FriendsOfFlarum/sentry@8fc6928#r34287868)

[luceos]

One thing I think is a must-have is the ability to easily enable Sentry. This would especially be helpful for taking immediate action when encountering unexpected errors on discuss. I will gladly provide an extension if need be.

[franzliedke]

I'd be most interested in your opinion on this:

IMO, there is no need for NotFound and similar exceptions to ever be handled with the fallback error handling (Whoops or pretty pages) - we are just using the exception mechanism to handle known error cases.

[luceos]

Catching NotFound exceptions allows for customising the returned response, eg with a decent error page or through extensions to list related discussions. Unless of course an extension could be configured to handle an uncaught exception (add to the handler stack, I think this is currently the case).

I would be very interested to know whether we can create decent error pages for any HttpException (with 500 probably being the 'exception').

[franzliedke]

@luceos Yes, I want to keep that part. My question was about the Whoops error page, which is currently shown (in debug mode) for NotFound exceptions and the like. IMO, these exceptions should always be handled by the error handler stack.

Debug mode (a.k.a. Whoops error pages) would then only have an effect for generic 500 errors.

[luceos]

My question was about the Whoops error page, which is currently shown (in debug mode) for NotFound exceptions and the like. IMO, these exceptions should always be handled by the error handler stack

When in debug mode you should always be able to get as much information as possible immediately (without diving into logs). I can imagine it would be easier to identify the cause of an incorrect 404 with whoops enabled for those kind of exceptions. Such exceptions can not only be caused by a firstOrFail but also by middleware, global scopes or random abort() calls.

[dsevillamartin]

If Whoops was kept for non-500 errors, which I agree with @luceos in that it would help debug, IMO it should still return the error code as the HTTP status.
Currently it only returns 500, which makes 404 errors look like server errors and can scare me a bit sometimes 😂

[franzliedke]

If Whoops was kept for non-500 errors, which I agree with @luceos in that it would help debug, IMO it should still return the error code as the HTTP status.

That is a great suggestion and would alleviate most of my concerns.

[franzliedke]

Now that #1648 is merged, what's left here is a rethink of how logging, exception handling and exception rendering fit together.

This also raises the question of how the new CSRF token mismatch handling fits in - generating the same status code from different exceptions is now possible, which means that we probably want to vary the returned pretty view based on the exception type, not the status code.