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

[Bug]: Infinite recursion with unhandled exception due to restore_exception_handler #81

Open
1 task done
gaambo opened this issue Jul 23, 2024 · 6 comments
Open
1 task done
Labels

Comments

@gaambo
Copy link

gaambo commented Jul 23, 2024

Description of the bug

I'm running into an infinite recursion when throwing an exception somehwere.
The PhpErrorController:on_exception exception handler is correctly called and logs the exception, but it then calls restore_exception_handler and re-throws the Exception which triggers the same function again.
According to a comment in the PHP docs:

Note that this does not work within an exception handler.

Therefore it runs the same method over and over again.

Reproduction instructions

  1. Bootstrap the default configuration
  2. Throw an exception somewhere in the code (e.g. on init)
  3. See how the request times out because of maximul call stack size and the log is full of the same error message
  4. alternative to 3: step-debug with xdebug and see the on_exception method get called over and over again

Expected behavior

In my case I just wanted to test the logging by manually throwing an exception. But when a real one throws, it loggs a huge amount of the same message and runs into fatal error with "Maximum call stack size".

I found out that you can use set_exception_handler(null); to reset it to the PHP defaults exception handler. Then the on_fatal will be called, because the exception is unhandled.

Another possibility would be to save the return-value of set_exception_handler (= the previous one) in Controller::log_php_errors and restore to it later.

Environment info

  • Package version: v1.2.0
  • Monolog: 1.27.1
  • PHP: 8.3
  • WordPress 6.5

Relevant log output

Fatal error: Uncaught Error: Maximum call stack size of 8339456 bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion? in /var/www/html/vendor/inpsyde/wonolog/src/Data/Log.php:222 Stack trace: #0 /var/www/html/vendor/inpsyde/wonolog/src/Data/HookLogFactory.php(104): Inpsyde\Wonolog\Data\Log->level() #1 /var/www/html/vendor/inpsyde/wonolog/src/Data/HookLogFactory.php(88): Inpsyde\Wonolog\Data\HookLogFactory->maybe_raise_level(0, Object(Inpsyde\Wonolog\Data\Log)) #2 /var/www/html/vendor/inpsyde/wonolog/src/Data/HookLogFactory.php(38): Inpsyde\Wonolog\Data\HookLogFactory->extract_log_objects_in_args(Array, 0) #3 /var/www/html/vendor/inpsyde/wonolog/src/LogActionSubscriber.php(67): Inpsyde\Wonolog\Data\HookLogFactory->logs_from_hook_arguments(Array, 0) #4 /var/www/html/web/wp/wp-includes/class-wp-hook.php(324): Inpsyde\Wonolog\LogActionSubscriber->listen(Object(Inpsyde\Wonolog\Data\Log)) #5 /var/www/html/web/wp/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters('', Array) #6 /var/www/html/web/wp/wp-includes/plugin.php(517): WP_Hook->do_action(Array) #7 /var/www/html/vendor/inpsyde/wonolog/src/PhpErrorController.php(116): do_action('wonolog.log', Object(Inpsyde\Wonolog\Data\Log)) #8 [internal function]: Inpsyde\Wonolog\PhpErrorController->on_exception(Object(Exception)) #9 {main} thrown in /var/www/html/vendor/inpsyde/wonolog/src/Data/Log.php on line 222

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@gaambo gaambo added the bug label Jul 23, 2024
@gaambo
Copy link
Author

gaambo commented Jul 23, 2024

I've got a simple workaround running, by extending PhpErrorController: https://gist.github.com/gaambo/6ea135cdc28fe3b30e61c2713db78040

I still need to test this further, I'm just getting started with Wonolog.

But if this works and it's in your interest, I can draft up a PR.

@gmazzap
Copy link
Contributor

gmazzap commented Jul 25, 2024

Thanks @gaambo

It never worked restoring the previous handler, but before PHP 8.3, it did not trigger an infinite loop either.

In the comment in the official docs you linked, where the exception is re-thrown, there is no mention of an endless loop because that is a new thing.

  • In PHP < 8.3.0, there's no endless loop. The handler is executed, and when the exception is re-thrown, the default PHP handler is executed.
  • In PHP > 8.3.0 < 8.3.5 there's no endless loop. The handler is executed, and then when the exception is re-thrown, it is executed again a second time, but it stops there.
  • In PHP >= 8.3.5, there's an endless loop.

This is a bug, but it is only visible on PHP 8.3, which we are yet testing for...

If you can do a PR with the fix, I'm definitively interested, but I would like to avoid the requirement that Php_Error_Controller::register() must be called before bootstrap () for it to work.

@gmazzap
Copy link
Contributor

gmazzap commented Jul 25, 2024

Moreover, it looks like you're using Wonolog v1. You should look into v2 for more modern PHP support, or even in the upcoming v3

@gmazzap
Copy link
Contributor

gmazzap commented Jul 25, 2024

It might be something like this: https://3v4l.org/jme3m

The Configurator::setupPhpErrorListener() method in that 3v4l above would be here in Wonolog: https://github.com/inpsyde/Wonolog/blob/3.x/src/Configurator.php#L1013

@gaambo
Copy link
Author

gaambo commented Aug 6, 2024

It never worked restoring the previous handler, but before PHP 8.3, it did not trigger an infinite loop either.

Very interesting finding, thank you @gmazzap.

If you can do a PR with the fix, I'm definitively interested, but I would like to avoid the requirement that Php_Error_Controller::register() must be called before bootstrap () for it to work.

That is just for my custom implementation for Php_Error_Controller, because the setup happens in the constructor. If it's in the framework, that's not required. I also just wanted to share the workaround for anyone coming here.

Moreover, it looks like you're using Wonolog v1. You should look into v2 for more modern PHP support, or even in the upcoming v3

I wasn't sure about their stability and therefore stuck to v1. But we'll probably want to use a 3rd party handler as well that requires at least monolog v2, so we'll probably update soon.

Should I create a PR for v1, v2 or v3? 😬

@trsteel88
Copy link

Having the same problem. Is a fix going to be implemented for this?

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

No branches or pull requests

3 participants