Unable to disable plugin from theme about bugsnag-wordpress HOT 23 CLOSED

Hi @fiskhandlarn, there's some higher priority work going on at the moment, but this, and more of a general update to this notifier, are in the pipeline, and I will update you when I know more.

from bugsnag-wordpress.

Hi @fiskhandlarn, from some preliminary tests it seems that after adding your filter the Bugsnag exception and error handlers are no-longer being set. Are you saying this is not what you are seeing?

from bugsnag-wordpress.

@Cawllec They are not disabled for me in version 1.3.0. If I rewrite the plugin so that the filter is being run when set in functions.php I still get the warnings in Error.php. What version are you using?

from bugsnag-wordpress.

This is with 1.3.0 as well. I'll have a further look into what you're reporting and get back to you.

from bugsnag-wordpress.

@Cawllec Strange. The only occurrence of bugsnag_set_error_and_exception_handlers I can find is this line in bugsnag.php:

            $set_error_and_exception_handlers = apply_filters('bugsnag_set_error_and_exception_handlers', true);

Which is called directly when the plugin is loaded, which happens before functions.php is even loaded.

Did you put your add_filter() call in your theme's functions.php?

from bugsnag-wordpress.

I did.

from bugsnag-wordpress.

@Cawllec When/where is the bugsnag_set_error_and_exception_handlers filter applied in your bugsnag.php?

For me it looks like this:

Line 314 (run when the plugin is included):
$bugsnagWordpress = new Bugsnag_Wordpress();

Line 33 (inside __construct()):
$this->activateBugsnag();

Line 70 (inside activateBugsnag()):
$this->constructBugsnag();

Line 88 (inside constructBugsnag()):
$set_error_and_exception_handlers = apply_filters('bugsnag_set_error_and_exception_handlers', true);

from bugsnag-wordpress.

So my path is going:
314: $bugsnagWordpress = new Bugsnag_Wordpress();
33: $this->activateBugsnag();
70: $this->constructBugsnag();
88: $set_error_and_exception_handlers = apply_filters('bugsnag_set_error_and_exception_handlers', true);
Similar to yours.

As for the filter, at the bottom of functions.php I've added:

add_filter('bugsnag_set_error_and_exception_handlers', function() {
    return true;
});

from bugsnag-wordpress.

@Cawllec Well yes, bugsnag's error handling is enabled with your code. But not because you're adding the filter in your functions.php, but because this is the default behaviour; as seen in bugsnag.php here:

$set_error_and_exception_handlers = apply_filters('bugsnag_set_error_and_exception_handlers', true);

As you can see, $set_error_and_exception_handlers is set to true by default.

What I'm asking for is the abilitiy to disable bugsnag's error handlers (as specified in the subject of this issue). What happens when you instead use this code in your functions.php?

add_filter('bugsnag_set_error_and_exception_handlers', function() {
    return false;
});

(Please notice the return false above instead of your return true.)

from bugsnag-wordpress.

Sorry, I meant return false. They are being disabled with the return false in the filter.

from bugsnag-wordpress.

@Cawllec Ah, alright :)

But I cannot for the life of me understand why your filter works and mine don't. The only way I can think of is if your functions.php is being loaded before bugsnag.php. Can you please confirm whether this is the case? (By echoing something at the top of both files for example.)

And maybe you can dump $set_error_and_exception_handlers just after line 88 with your false-filter active?

from bugsnag-wordpress.

So with a print_r at the top of functions.php and bugsnag.php, and with the

add_filter('bugsnag_set_error_and_exception_handlers', function() {
    return false;
});

and a var_dump on $set_error_and_exception_handlers the output I get is:

Loaded functions.php
Loaded bugsnag.php
bool(false)

(with newlines added by me).

So it seems that your Bugsnag.php may be being loaded prematurely? What wordpress version are you running?

from bugsnag-wordpress.

@Cawllec It was 4.9.4, but I've updated to 4.9.5 now (with the same behaviour).

I haven't seen that loading order before. https://wordpress.stackexchange.com/questions/26537/between-functions-php-widgets-and-plugins-which-is-loaded-first/26622#26622 also states that plugins are loaded before the theme.

Are you running "vanilla" WordPress?

from bugsnag-wordpress.

I'm using the Wordpress 4.9.4 docker image , running on PHP 7.2. It should be as unmodified as possible, I'll check and see if the image has any differences.

from bugsnag-wordpress.

@Cawllec Please do :)

If your WordPress files look like this:
https://github.com/WordPress/WordPress/blob/master/wp-settings.php#L304
https://github.com/WordPress/WordPress/blob/master/wp-settings.php#L433

... the theme should be loaded after all plugins.

Just to rule out any confusion, you are using the filter in the functions.php in a theme, right?

from bugsnag-wordpress.

I absolutely was not. You're quite right about loading order - bugsnag.php loads before theme/functions.php meaning the add_filter function has no effect there.

I misread your point very early on, sorry this has been a bit more roundabout than it should have been.
I understand your issue now, I'll create a ticket to have a look into the best way for us to solve this issue.

• @GrahamCampbell, do you have any input on potential fixes/solutions here?

from bugsnag-wordpress.

@Cawllec No worries, I'm only happy we understand each other now. :)

A solution for actually make the filter usable is to call activateBugsnag in a later stage ('init' for example). But then bugsnag wouldn't report any bugs happening before then. The best solution would therefore probably be to turn on the error handling as soon as the plugin loads and then remove it if requested later.

But, I have no idea how to fix the plugin from causing these warnings when the error handlers once are disabled:

Warning: strpos(): Empty needle in plugins\bugsnag\bugsnag-php\Error.php on line 386

from bugsnag-wordpress.

@Cawllec @GrahamCampbell Any news on this? :)

from bugsnag-wordpress.

Hi @fiskhandlarn, it's been a while but I've had a chance to look into this issue again. The strpos issue you are seeing seems to be related to the filters that've been setup. Are you setting any filters in your configuration?

from bugsnag-wordpress.

I'm going to close this issue for now, but if the issue persists please get in touch.

from bugsnag-wordpress.

@Cawllec The strpos issue isn't really the real issue IMO. The original issue (that the bugsnag_set_error_and_exception_handlers filter is unusable) still persists (as confirmed by you in #27 (comment)). Please reopen.

from bugsnag-wordpress.

The ticket to fix this is being prioritised, but will likely be included with other notifier improvements. The solution as it stands is to use restore_error_handler and restore_exception_handler conditionally and avoid using blank string filters '' so the strpos warning isn't flagged up.

As work on the notifier improvements are implemented this thread will be brought back in.

from bugsnag-wordpress.

@Cawllec Alright, I will use restore_error_handler and restore_exception_handler.

FYI: if the "bugsnag_filterfields" option isn't set in the database $this->filterFields (in Bugsnag_Wordpress class) will be set to false which leads to the strpos warning. Bugsnag_Wordpress::filterFields() doesn't account for this. Although, I can now see that this happens when our theme sets "bugsnag_api_key" and "bugsnag_notify_severities" options in our custom install.php file without setting the "bugsnag_filterfields" option.

from bugsnag-wordpress.