New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separated error and exception handling from logging #6842
Conversation
… logging - the error handler logs warnings and notices, and turns errors into exceptions (`ErrorException`) - the exception handler catches all uncatched exceptions and display them to the user (HTML or CLI) - the "screen" logging backend has been removed - I've normalized exceptions/errors shown to the user in HTML (wether they are catched by the FrontController or not)
…L notifications The "screen" backend (WebNotificationHandler) now logs to HTML notification boxes using the `Notification` api. I've re-set the default log level to WARN, and logged PHP notices/warnings to "warning" so that on a default install they are shown in the page in a yellow/warning notification box. For the record, the default log level had previously been changed from WARN to ERROR because screen logging was previously messing the HTML/JSON output (which was breaking archiving). With the logger refactoring this is no longer a problem.
I've updated the PR:
I've reviewed with @mattab, I'm merging this in the Logger refactoring branch. If you have any more comment, feel free to add them here or on #6813. |
Cannot add tests for the screen backend because I would need to mock the session, which is not possible right now because it's all static stuff.
Separated error and exception handling from logging
Looks good @mnapoli though i have feedback:
|
@mattab Where/in which case should I display that? |
FYI it was displayed for errors (the yellow box), and it's still displayed for those errors |
@mnapoli For |
@mattab done |
This is an implementation of the changes I suggested in #6813: separating logging and exception handling.
I'm opening it as a separate pull request so that we can discuss it here (wether we want this or not) and I can post screenshots without flooding the other pull request.
Summary
ErrorException
)ErrorException
) and display them to the user (HTML or CLI)Notification
class): it doesn't hijack/break HTML/JSON output anymoreScreenshots
Uncatched exception in a controller/API/…
Same as before:
Uncatched exception outside of the dispatch loop
Before:
Now:
Error
Was:
Now looks like this:
Fatal error
Same as before:
Custom message logged when
screen
logging is enabledExample code:
Before:
(the lines of log where added at the top of the page)
Now:
(we use the notification API)