Skip to content
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

Enhance errors handling #18046

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

cedric-anne
Copy link
Member

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

This PR contains multiple changes, but each of them is related to a better handling of errors.

Improvements of the error page

See Enhance errors rendering commit.

The rendering of errors made by the ErrorController has been enhanced to:

  • display a title/message that depends on the error type, is translated, and can be customized by the exception thrower,
  • have a layout adapted to the session context;
  • have the stacktrace available when the debug mode is active (was only available for development env before this PR).

before:
image

after:
image

Better handling of exceptions in the legacy front files

See Catch exceptions thrown when sending a streamed response commit.

While testing my changes, I figured out that an exception thrown from inside a legacy script was not forwarded to the error controller. Unless I miss something, this is due to the fact that exception are not caught during the response sending.
To fix this, I encapsulated the response sending in a try/catch block to be able to forward the exceptions thrown when response is streamed to the error controller.

Removal of the legacy error displaying methods

See Replace error displaying methods by an exception throwing commit.

I replaced the usages of the Html::displayErrorAndDie(), Html::displayNotFoundError() and Html::displayRightError() by an exception throwing. It simplifies the code, remove many exit/die usages, and permits to use the new error handling system for the corresponding errors.

Technical changes

I introduced a new \Glpi\Exception\Http\HttpExceptionInterface interface that permit to defines a message to display (the message shown to the end user, that must be translated) that differs from the error message (the technical message that can be find in the error backtrace, that must not be translated).
I extend some of the Symfony\Component\HttpKernel\Exception exceptions to implement this interface.

I also moved some login into a AccessErrorListener class to centralize the redirections made on specific error cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant