bugsnag / bugsnag-wordpress

BugSnag error monitoring for WordPress sites
http://wordpress.org/plugins/bugsnag/
GNU General Public License v2.0
20 stars 15 forks source link

Add a constant to control setting error handlers #45

Closed imjoehaines closed 4 years ago

imjoehaines commented 4 years ago

Goal

With the Bugsnag exception handlers enabled, errors aren't output to the browser window (see #41)

The current method of disabling the Bugsnag requires using a bugsnag_set_error_and_exception_handlers filter but, because we want to initialise Bugsnag ASAP, this filter runs before a theme's functions.php file. This makes it less then ideal as you need to edit the core functions.php file in order to disable the exception handlers

This PR adds a nicer way to control whether the exception handlers are set by using a constant, which can be defined in wp-config.phpBUGSNAG_SET_EXCEPTION_HANDLERS. If this is set to true then the PHP exception and error handlers will be set

It's important to note that we will still report errors with BUGSNAG_SET_EXCEPTION_HANDLERS set to false. However this is done using a PHP shutdown function, which means we have less context about any errors that may occur — for example, we don't currently display an exception backtrace in the shutdown function, because at that point it's been converted to a PHP Fatal Error

I've kept the filter around for BC reasons, but it will now filter the constant's value if defined. If it's not defined then the current default of true will be filtered (i.e. nothing has changed if the constant is not defined)

Changeset

Tests

This was tested manually by defining a BUGSNAG_SET_EXCEPTION_HANDLERS constant with a value of true or false in wp-config.php:

// Disable Bugsnag exception handlers so PHP reports exceptions inline
define('BUGSNAG_SET_EXCEPTION_HANDLERS', false);

If the constant isn't set (or is set to true) the exception handlers should be enabled

Discussion

Alternative Approaches

Outstanding Questions

The interaction between the new constant and the existing filter is perhaps unintuitive. I think this is the clearest option that also preserves BC, but there are others, such as using the constant if it's defined and otherwise falling back to the filter

We could also drop the filter entirely, but we can't tell how many people are currently using it

Linked issues

Related to #41 and #44

Review

The constant was added in https://github.com/bugsnag/bugsnag-wordpress/commit/7cd7086a023637589bdd86a81189cce1080d60be, the rest of the changes are fixes for StyleCI

For the submitter, initial self-review:

For the pull request reviewer(s), this changeset has been reviewed for: