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

Bugsnag shouldn’t report silenced errors #35

Open
gchtr opened this issue Jan 24, 2019 · 4 comments
Open

Bugsnag shouldn’t report silenced errors #35

gchtr opened this issue Jan 24, 2019 · 4 comments
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug

Comments

@gchtr
Copy link

gchtr commented Jan 24, 2019

Expected behavior

When error reporting is set to Warning, then Bugsnag shouldn’t report silenced PHP errors. For example, the following code shouldn’t report a warning:

@unlink( '/path/to/inexistent/file.jpg' );

Observed behavior

Bugsnag reports PHP warnings for code that is silenced with an @.

Steps to reproduce

  1. Put this code in functions.php of your theme and reload the page.
    @unlink( '/path/to/inexistent/file.jpg' );
  2. See error in the dashboard.

I’m not sure, but this logic might be what causes the warning to be reported:

https://github.com/bugsnag/bugsnag-php/blob/e50a26a5535f5e4ffb386a449710f2ce9c050287/src/Configuration.php#L543-L550

PHP’s documentation for error control operators clearly states:

If you have set a custom error handler function with set_error_handler() then it will still get called, but this custom error handler can (and should) call error_reporting() which will return 0 when the call that triggered the error was preceded by an @.

In shouldIgnoreErrorCode(), the function might return without checking error_reporting(). So I’m guessing that this is why silenced errors are reported.

Version

Bugsnag: 1.3.1
WordPress: 5.0.3

Additional information

Tell me if you need some more information.

@Cawllec
Copy link
Contributor

Cawllec commented Jan 24, 2019

Thanks for the report @gchtr. It's an issue where if you've set an errorReportingLevel, we only check that, and not the error_reporting method. We're aware of the issue and will release a fix as soon as we can.

As a temporary fix, you could create a callback function that returns false if error_reporting is 0.
Something like:

$bugsnagWordpress->setBeforeNotifyFunction(function ($report) {
  if (error_reporting() === 0) {
    return false;
  }
});

@abigailbramble abigailbramble added backlog We hope to fix this feature/bug in the future bug Confirmed bug labels Oct 3, 2019
@fiskhandlarn
Copy link
Contributor

Any news on when bugsnag/bugsnag-php#537 can be made available in bugsnag-wordpress (i.e. bump the bundled https://github.com/bugsnag/bugsnag-php/ from 2.10.0 to at least 3.18.0)?

@mattdyoung
Copy link

Hi @fiskhandlarn

bugsnag-wordpress will need a fair amount of rework to pull in bugsnag-php v3.x. We don't have a timeframe yet but hope to do this when priorities allow.

For the time being the suggested workaround of modifying the behavior in a callback should still work.

@fiskhandlarn
Copy link
Contributor

if anyone's interested, i'm using this to at least silence lines that begins with @:

add_action('plugins_loaded', function() {
    global $bugsnagWordpress;
    if (isset($bugsnagWordpress) && is_callable([$bugsnagWordpress, 'setBeforeNotifyFunction'])) {
        try {
            // Bugsnag reports PHP warnings for code that is silenced with an @.
            // https://github.com/bugsnag/bugsnag-wordpress/issues/35#issuecomment-457130639
            $bugsnagWordpress->setBeforeNotifyFunction(function ($report) {
                $stacktrace = $report->stacktrace->toArray() ?? null;
                if ($stacktrace) {
                    $line = trim($stacktrace[1]['code'][$stacktrace[1]['lineNumber']] ?? '');
                    if (strpos($line, '@') === 0) {
                        return false;
                    }
                }
            });
        } catch (\BadMethodCallException $e) {
            // bugsnag api key probably not set, ignore
        }
    }
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

5 participants