Use error handlers rather than the error suppression operator

[rmccue]

In various places in core (and included libraries, such as pclzip, FTP and phpass), the error suppression operator is used to suppress warnings and notices. However, this also suppresses fatal errors, which makes it a huge pain for debugging.

The big one here is fopen, where it is used to suppress warnings when opening a handle fails. For normal files, this is fine since the only errors generated are warnings. When using streams and custom stream handlers, these can generate errors which are really hard to find with this.

(I ran into this with the App Engine plugin, where the SDK method generates a fatal error if the mode is a, which is used with error suppression in win_is_writable().)

[mark-k]

In what way an error handler is better? If I feel that something funny is going on I just look at the error log file. How will having an error handler improve anything?

+1 to getting rid of all suppression and trying to actually handle all exceptions (even if by just emitting an error under WP_DEBUG) but I don't think this is the way to get there.

[johnbillion]

Replying to mark-k:

In what way an error handler is better? If I feel that something funny is going on I just look at the error log file. How will having an error handler improve anything?

As Ryan stated, fatal errors are also suppressed when the @ operator is used. This is a complete nightmare when your script dies for no apparent reason. The only option then is to run Xdebug with xdebug.scream set to true. It works, but it's far from ideal.

We should audit core in its entirety and remove as many @ operators as possible. The remaining ones should only be used when WP_DEBUG isn't set to true (​example).

I'll be willing to do this at some point. I just lost an hour due to a fatal error that was being suppressed.

[Howdy_McGee]

Doing a quick search on this, there are at least ~200 instances of the @ suppression operator across ~55 files. Maybe to make this process easier we should split it up (by ticket) into smaller bites either by component or file. Most files have less 1 or 2 instances but the class-pclzip.php file has around 60 instances.

[Howdy_McGee]

Actually, there's around ~400 and I generated a doc to track this if this is something we want to get done. ​Suppressed Errors Google Doc

[SergeyBiryukov]

Replying to Howdy_McGee:

Doing a quick search on this, there are at least ~200 instances of the @ suppression operator across ~55 files. Maybe to make this process easier we should split it up (by ticket) into smaller bites either by component or file. Most files have less 1 or 2 instances but the class-pclzip.php file has around 60 instances.

Thanks for the research! I think it would make sense to split this into smaller tickets per file or a group of related files.

These files were previously external libraries:

wp-admin/includes/class-ftp-pure.php
wp-admin/includes/class-ftp-sockets.php
wp-admin/includes/class-ftp.php
wp-admin/includes/class-pclzip.php
wp-includes/IXR/*
wp-includes/class-phpass.php

With the upstream sources seemingly abandoned, they are now treated as "adopted" rather than external, per comment:1:ticket:49163 and comment:2:ticket:48267. That said, I think we should still minimize changes to them and probably exclude them from the first pass. So far, they only received some necessary changes like [32990] / #31982, or minor typo fixes like [47123] / #49163.

These are supported external libraries that should definitely be excluded from any changes here:

wp-includes/ID3/*
wp-includes/random_compat/*
wp-includes/SimplePie/*
wp-includes/class-phpmailer.php
wp-includes/class-pop3.php
wp-includes/class-requests.php
wp-includes/class-smtp.php

[SergeyBiryukov]

In 47617:

Code Modernization: Remove error suppression from parse_url() calls.

Previously, the @ operator was used to prevent possible warnings emitted by parse_url() in PHP < 5.3.3 when URL parsing failed.

Now that the minimum version of PHP required by WordPress is 5.6.20, this is no longer needed.

Props netpassprodsr, Howdy_McGee.
Fixes #49980. See #24780.