stayallive / wp-sentry

A (unofficial) WordPress plugin reporting PHP and JavaScript errors to Sentry.
https://wordpress.org/plugins/wp-sentry-integration/
MIT License
305 stars 48 forks source link

Errors in Sentry or setBeforeSendCallback silently dropped? #138

Closed kkmuffme closed 2 years ago

kkmuffme commented 2 years ago

I have both a custom set_error_handler and set_exception_handler which are loaded before WP (via auto_prepend_file) to catch all errors and handle them before Sentry is loaded by WP mu-plugin.

However it seems, that Sentry internally handles all errors (including fatals) from setBeforeSendCallback or the SDK itself and just dropping it silently. They never show up in my error handler.

This makes it extremely awkward, as those errors are never reported anywhere. Is this something we can fix as part of WP Sentry or should I open an issue in the Sentry PHP SDK repo?

stayallive commented 2 years ago

It makes sense from a looping perspective where bubbling out errors from the SDK can result in an infinite loop, so although not ideal the Sentry SDK swallowing it's own errors makes sense.

We do capture the old exception handler (https://github.com/getsentry/sentry-php/blob/877bca3f0f0ac0fc8ec0a218c6070cccea266795/src/ErrorHandler.php#L207) and try to forward any to there (https://github.com/getsentry/sentry-php/blob/877bca3f0f0ac0fc8ec0a218c6070cccea266795/src/ErrorHandler.php#L350-L377) so maybe there is a bug there that's preventing things from ending up in your handler?

It's also possible that WordPress and Sentry and your handler are fighting and a handler is getting lost at some point. Maybe for you use case it's better to not load Sentry at all via the plugin but via your own auto_prepend_file so we stay out of it's way and you can forward any error to Sentry from there? Multiple error handlers is a bit of a mess in PHP land.

kkmuffme commented 2 years ago

My handler (from the auto_prepend_file) is called just fine (for all errors, including the ones Sentry handles, in which case I abort, since Sentry already got them). However the Sentry native errors never show up in the custom handler.

But I guess where/why there is a potential loop problematic. Would be better if it wouldn't be caught immediately though, but just runs the normal course and Sentry filters the error (that came from itself) out before sending or something.