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

Respect the result of any user-defined error handling #437

Merged
merged 2 commits into from
Mar 3, 2019

Conversation

ryanshoover
Copy link
Contributor

@ryanshoover ryanshoover commented Feb 28, 2019

Current Situation

Currently the ErrorHandler will call any user-defined set_error_handler after Rollbar has processed all if its needs. However, if Rollbar decides to stop handling the error (if no logger is registered or the error is below the logger's base level) then the user's error handler is never called.

Proposed Solution

The proposed solution will first call the user-defined set_error_handler. If the user's function returns a truthy value then Rollbar stops handling the error. A returned truthy value tells PHP to stop handling the error and is a way for user-defined error handlers to circumvent PHP's default methods.

With this solution, any user-defined error handlers can suppress error messages and Rollbar won't pass them upstream.

Impetus

This PR comes from efforts to suppress a known warning triggered by WordPress in a PHP 7.2 environment. Our custom error handler to keep a harmless warning from flooding client sites is being ignored by Rollbar's error handler.

@jessewgibbs
Copy link
Contributor

@ryanshoover thanks for the PR.

@ArturMoczulski could you review when you have a chance?

@ryanshoover
Copy link
Contributor Author

The failed Travis build seems to be a separate issue from my PR?

The command "composer install --no-interaction" failed and exited with 255 during .

I didn't touch composer dependencies

@ArturMoczulski
Copy link
Contributor

@ryanshoover great catch! Merging the PR right now.

@ArturMoczulski ArturMoczulski merged commit 4745370 into rollbar:master Mar 3, 2019
@ArturMoczulski
Copy link
Contributor

Released in v1.7.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants