`trigger_error()` errors not being logged to Sentry when also running the Query Monitor plugin

[lubo-makky]

I have setup the plugin as adviced including must-use-proxy in wp-content/mu-plugins dir.

Works well for general errors and unhandled exceptions, however if I use throw_error() nothing is being logged in Sentry, only in server log.

In my wp-config.php:

define( 'WP_SENTRY_DSN', 'https://.....@....' );
define( 'WP_SENTRY_ERROR_TYPES', E_ALL );
define( 'WP_SENTRY_SEND_DEFAULT_PII', true );
define( 'WP_SENTRY_PUBLIC_DSN', 'https://.....@....' );
define( 'WP_SENTRY_ENV', 'production' );

Exception gets logged:

add_action( 'wp_head', function() {
    throw new \ErrorException('Hello World');
}, 1 );

However trigger_error doesn't:

add_action( 'wp_head', function() {
    trigger_error('Hello World', E_USER_WARNING); 
    // tried also E_USER_NOTICE, E_USER_ERROR flags
}, 1 );

Thank you in advance for any help.

[lkraav]

I'm also noticing some things never getting logged even with with mu-plugin. Haven't been able to identify what the pattern is, yet.

[lubo-makky]

I have figured out what was wrong. I use Query Monitor plugin for debugging purposes. This plugin was causing the trouble. After deactivating it errors from trigger_error() are getting through to Sentry.

Maybe a line in readme could help.

[stayallive]

Happy to accept a PR to add that to the README.

Although of you have query monitor running (a debug tool) it might be expected error reporting is a bit wonky at best.

Unfortunately there are also still a lot of plugins who think it’s their job to mess with the error reporting to hide their crappy programming 🙈 (not talking about query monitor and the like since those have a good reason to mess with error reporting). So this could also be a cause for broken reporting.

[stayallive]

To prevent leaving this open I'll close it, if you want to add something to the readme still I'm happy to accept a PR for that but I still believe it is expected that debug tools might interfere with Sentry's ability to track issues accurately.

[lubo-makky]

Will make a PR shortly, sorry for leaving this issue open. Thanx for your your replies.

[lkraav]

I can confirm, disabling Query Monitor increases the amount of things Sentry is able to catch. For example, in my case, admin-ajax requests. Can we reopen this?

@johnbillion could you offer insight here :pray: ? Are we somehow crashing error handlers into each other, or?

Most certainly we'd ideally like to keep both plugins running at all times.

[johnbillion]

Breaths in...

Aaaaaaarrrrrrrggggghhhhhhhhhhh.

The error handling in QM is quite ugly because it has to battle with every version of PHP back to 5.2, while simultaneously ensuring that various flavours of fatal errors, throwables, and exceptions do not get hidden, working around the WSOD protection in WordPress core, ensuring that the default error handler (eg. the error logger in PHP) still works, loading early enough to catch all errors, and more.

(Getting off topic now, but the catchable fatal error handling that was introduced in PHP 7 does not cover every type of fatal, so QM still needs some workarounds for that. Even popular error handling libraries such as Whoops don't get this completely correct.)

Its error handler always returns boolean false. My understanding is that this means any error that gets caught by QM will still be passed onto subsequent error handlers, such as the default PHP error logger, but after reading the docs for set_error_handler() again maybe this behaviour applies only to the built-in handler and not other registered handlers.

Does Sentry register an error handler with set_error_handler()?

[stayallive]

@johnbillion I'm a bit fuzzy on the subject matter (because it's a mess) but yes set_error_handler is used in Sentry, however, using multiple error handlers in PHP is a hot mess.

From the set_error_handler docs:

It is important to remember that the standard PHP error handler is completely bypassed for the error types specified by error_types unless the callback function returns FALSE.

So the returning false only applies to the PHP internal handler (logging to error_log etc.).

Also set_error_handler is singular in it's name and function, you cannot set multiple error handlers (calling set_error_handler returns the "previous" handler), many libraries like Sentry try to work around this be storing the previous handler internally and (if configured) calling that "previous" handler. It is possible that QM does not do this or doesn't work correctly with QM or it might even be the other way around (it matters in which order QM loads and Sentry does).

---

It might be a good plan to try and get a minimal reproducible WordPress installation (just Sentry & QM + a trigger_error or something) where the error is caught by QM but not by Sentry so we can investigate in a clean environment.

I am still against having multiple error handler plugins running (because of this reason) so I'm not very invested in this but if any of you are willing to create such an example I would be happy to try and see if I can make sense of the issue and possibly find a way to fix this (either in QM or the Sentry plugin).

[johnbillion]

Sentry try to work around this be storing the previous handler internally and (if configured) calling that "previous" handler.

QM does this for its exception handler, but not for its error handler. That could well be the problem. I will have to take a look back through QM's history of changes to this handling in case there's an explicit reason it does not do this.

[stayallive]

Ah. That might be the “problem” indeed. If that would indeed be the root cause there is nothing we can do except for re-registering after QM does which is a possibility, but it would be nicer if QM keeps the chain going, unless they have a good reason not to.

[lkraav]

Until we figure something out here, I think define QM_DISABLE_ERROR_HANDLER true can work to retain majority of QM on-the-fly performance measurement functionality.

[stayallive]

I think we can for now close this issue, it's still searchable. If we need to do anything to improve compatibility let's open up a new issue so we can discuss with a fresh eye!

[lkraav]

The error handling in QM is quite ugly because it has to battle with every version of PHP back to 5.2, while simultaneously ensuring that various flavours of fatal errors, throwables, and exceptions do not get hidden, working around the WSOD protection in WordPress core, ensuring that the default error handler (eg. the error logger in PHP) still works, loading early enough to catch all errors, and more.

@johnbillion I assume QM doesn't need to be PHP 5.2 compatible anymore, correct?

Have any improvement paths opened here?

[johnbillion]

It still supports PHP 5.3, but the next major is going to drop support for PHP 5 altogether. No ETA just yet.