Skip to content

Don't silence fatal errors with @ #3685

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

Closed
wants to merge 1 commit into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Nov 26, 2018

When the silencing operator @ is used, the intention is generally to silence expected warnings or notices. However, it currently also silences fatal errors. As fatal errors also abort request execution, the result will often be a hard to debug white screen of death.

This patch changes the behavior of @ to only silence warnings, notices and other low-level diagnostics. The following error types will no longer be silenced:

  • E_ERROR
  • E_CORE_ERROR
  • E_COMPILE_ERROR
  • E_USER_ERROR
  • E_RECOVERABLE_ERROR
  • E_PARSE

There are two main implications for backwards compatibility:

  • Code that legitimately wants to silence fatal errors for whatever reason should now use error_reporting() (or ini_set()) to do so.
  • Error handlers that want to only handle non-silenced errors may have to be adjusted. A common pattern I found in our own tests if checks for error_reporting() != 0 to detect silencing. This should be changed to error_reporting() & $err_no to detect whether the specific error type is silenced.

@krakjoe
Copy link
Member

krakjoe commented Jan 31, 2019

Merged as 839bdf1

Thanks.

@krakjoe krakjoe closed this Jan 31, 2019
@nikic
Copy link
Member Author

nikic commented Jan 31, 2019

Sorry, this wasn't ready for merging yet, because more tests need to be fixed. I've reverted for now.

@nikic nikic reopened this Jan 31, 2019
@nikic nikic force-pushed the silence-of-the-fatals branch from e9570bf to cb1784c Compare February 5, 2019 14:39
@nikic nikic added this to the PHP 8.0 milestone Feb 11, 2019
@nikic
Copy link
Member Author

nikic commented Feb 11, 2019

Merged as a302d11.

@nikic nikic closed this Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants