WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 11 days ago

#24780 new defect (bug)

Use error handlers rather than the error suppression operator

Reported by: rmccue Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Filesystem API Keywords: needs-patch
Focuses: Cc:

Description

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().)

Change History (7)

#1 @toscho
7 years ago

  • Cc info@… added

#2 @johnbillion
7 years ago

  • Cc johnbillion added

#3 follow-up: @mark-k
7 years ago

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.

#4 in reply to: ↑ 3 @johnbillion
7 years ago

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.

#5 @chriscct7
4 years ago

  • Keywords needs-patch added

#6 @Howdy_McGee
3 weeks ago

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.

Last edited 3 weeks ago by Howdy_McGee (previous) (diff)

#7 @Howdy_McGee
11 days ago

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

Note: See TracTickets for help on using tickets.