-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Cleanup error handler calls for future migration #18696
base: main
Are you sure you want to change the base?
Conversation
2b4b38b
to
d9b0e80
Compare
d9b0e80
to
9ce9620
Compare
ajax/dashboard.php
Outdated
ErrorHandler::getInstance()->handleException($e, true); | ||
ErrorUtils::logException($e); | ||
ErrorUtils::outputExceptionMessage($e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there are multiple places where a single line is replaced with 2 new ones. That's not really easy to understand and may be confusing.
Also, many things have changed regarding errro management, maybe a £ explaining how we should handle them correctly in the developper documentation would be a good thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This double call is going to change. PR is still a draft.
I agree that the documentation should be update, I will take care of this once this PR will be finished.
9ce9620
to
ab5a171
Compare
e03b7d3
to
9f4fdb1
Compare
Final goal would be to leave error handling entirely to Symfony, and convert all existing "error handling" code in GLPI to either exceptions OR to
trigger_error
calls.This first cleanup is here to standardize this with the 4 current identified cases (error or exception, and for each either log and/or output), there might be a bit too much new code (it covers all cases, the goal is to have none of these cases in the future) but part of it will be in the new error handling system anyway