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

[ticket/16958] Add exception handler for handling uncaught exceptions #6361

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

marc1706
Copy link
Member

@marc1706 marc1706 commented Jan 30, 2022

This should improve the user experience a bit when it comes to exceptions being thrown while using phpBB. In addition to certainly having a nicer touch to them visually, they should prevent users of phpBB from ending up at a white page (error 500 style white page).

Checklist:

  • Correct branch: master for new features; 3.3.x for fixes
  • Tests pass
  • Code follows coding guidelines: master and 3.3.x
  • Commit follows commit message format

ToDo:

  • Check behavior on normal requests
  • Check behavior on AJAX requests
  • Check potential use in installer

Screenshots:

  • Debug disabled: image
  • Debug enabled: image
  • Debug enabled and display of arguments enabled in php.ini:
    image

Tracker ticket (set the ticket ID to your ticket ID):

https://tracker.phpbb.com/browse/PHPBB3-16958

@marc1706
Copy link
Member Author

marc1706 commented Feb 2, 2022

I've marked this as WIP for now as I still want to test the behavior on other pages.

@marc1706
Copy link
Member Author

Working on proper errors during ajax requests as well:
image

@3D-I
Copy link
Contributor

3D-I commented Feb 27, 2022

Working on proper errors during ajax requests as well

Also with debug enabled?

@marc1706
Copy link
Member Author

marc1706 commented Mar 3, 2022

Working on proper errors during ajax requests as well

Also with debug enabled?

In ajax requests I have opted to not add a backtrace.

@marc1706 marc1706 removed the WIP 🚧 label Apr 13, 2022
Copy link
Member

@Derky Derky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking good, but I've some testing results:

  1. SQL errors are HTML escaped twice in the gray box:

double_escaped_sql_error

  1. When using an incorrect database password I get a white page instead of an error message in this styling.

  2. If a PHP error occurs during an AJAX request (for example a variable doesn't exist on an object) the error is hidden with a "something went wrong" in the AJAX pop-up. But you can no longer see the actual error message if you open up the network tab to see the AJAX response. The original error should also be returned from this endpoint and ideally be shown in the pop-up as well.
    Original behaviour: (able to find the actual error message)

original_behaviour_ajax_backend_error

Current (this PR) behaviour: (impossible to find the actual error message)

this_pr_behaviour_ajax_backend_error png

@rubencm
Copy link
Member

rubencm commented Dec 9, 2022

The exceptions that are thrown from phpbb (runtime_exception for example), are multilanguage, since the message is a language variable and they have an additional constructor parameter for the language parameters. Maybe you could try translating them if the exception is an instance of runtime_exception or implements exception_interface.

One more thing, as I report here https://tracker.phpbb.com/browse/PHPBB3-16198 the component we are using to handle errors is discontinued, I don't know if it is worth developing on top of it, or better update it first

@marc1706 marc1706 changed the base branch from 3.3.x to master May 1, 2023 15:13
@@ -13,9 +13,12 @@

namespace phpbb\debug;

use Symfony\Component\Debug\BufferingLogger;
Copy link
Member

@rubencm rubencm May 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you can remove symfony/debug in composer.json

You should replace symfony/debug by symfony/error-component in composer.json

@rubencm
Copy link
Member

rubencm commented Sep 21, 2023

Update symfony/error-component to ^6.3 (#6528)

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