Make WP_Error extending Exception to make it throw-able

[hakre]

Does it makes sense to make the WP_Error class extending the standard PHP Exception so we can throw it when needed? Feedback appreceated.

[hakre]

Related: #16782 - Contains a patch with a WP_Exception class.

[scribu]

Closing as dup of #16782, to keep discussion in one place.

[hakre]

This is not a duplicate of #16782, re-opening to keep things apart preventing clutter

[hakre]

Making WP_Error extending Exception

[hakre]

Making WP_Error extending Exception (Updated)

[hakre]

I've attached a patch that extends WP_Error from Exception.

[westi]

Switching to an Exception and throwing it changes the whole way in which we use WP_Error.

I'm not convinced we need to do this.

• What is the benefit?
• How is this backwards compatible with old code?

[hakre]

Replying to westi:

Switching to an Exception and throwing it changes the whole way in which we use WP_Error.

Well actually prior to PHP 5 exceptions were not available but the suggestion given with the patch is not changing much so far, it's just extending the current.

I'm not convinced we need to do this.

• What is the benefit?

To have an WP based, throw-able error class. If you don't know what the benefit of a concrete exception class is, please let us know.

• How is this backwards compatible with old code?

I expect it to be 100% compatible. The WP_Error class can be instantiated as known and returned from functions. If you see some scenarios where this should not be the case, please report them in this ticket.

[filosofo]

We currently have poor-man's exceptions in a number of places. For example, ​here a WP_Error object is returned, but nothing in core WP does anything with that return value. So upon error the logic branches, an error object is created, but nothing can be done with the helpful information in the way that a thrown exception could be caught.

[westi]

Replying to filosofo:

We currently have poor-man's exceptions in a number of places. For example, ​here a WP_Error object is returned, but nothing in core WP does anything with that return value. So upon error the logic branches, an error object is created, but nothing can be done with the helpful information in the way that a thrown exception could be caught.

The real bug is that nothing actually checks for the WP_Error that is returned.

Switching to throwing the Error Object instead makes it harder to track error handling correctly in my experience.

Ideally everywhere we could return a WP_Error we should be checking for it in the calling code and anywhere we don't is a bug.

[hakre]

Replying to westi:

The real bug is that nothing actually checks for the WP_Error that is returned.

If you consider that a bug, please open a ticket. We can reference it here while keeping the discussion on topic. The suggestion made in this ticket is not about rewriting the code wherever WP_Error is in use.

This is not going to be constructive at the moment.

[filosofo]

Replying to westi:

The real bug is that nothing actually checks for the WP_Error that is returned.

One of the great things about thrown exceptions is that you can reduce the number of error checks. Currently we have an error-checking structure roughly like this:

function a (){
   $b = b();
   if ( is_wp_error( $b ) ) {
      return $b;
   } else {
      $c = c( $b );
      if ( is_wp_error( $c ) ) {
         return $c;   
      } else {
         echo $c->property;
      }
}
function b (){...}
function c (){...}

$result = a();
if ( is_wp_error( $result ) ) {
   echo $result->get_error_message();
}

With thrown exceptions, we never have to check whether the returned object is an error object. PHP takes care of that for us:

function a (){
   $b = b();
   $c = c( $b );
   echo $c->property;
}
function b (){...}
function c (){...}

try {
   $result = a();
} catch( WP_Exception $err ) {
   echo $result->getMessage();
}

The second example is easier to grok and does less work to achieve the same result.

Switching to throwing the Error Object instead makes it harder to track error handling correctly in my experience.

One simple thing we can do to address that is have different exception classes for different code purposes, and throw the relevant kind of exception:

class WP_DB_Exception extends WP_Exception {}
class WP_Taxonomy_Exception extends WP_Exception {}
class WP_Capabilities_Exception extends WP_Exception {}

So you might show a WP_DB_Exception message only when in debug mode, but always show the WP_Capabilities_Exception. (These aren't great examples but you get the idea.)

And there is the Exception getTrace method if you really want to track the origin of the Exception in the code.

[westi]

Replying to filosofo:

Replying to westi:

The real bug is that nothing actually checks for the WP_Error that is returned.

One of the great things about thrown exceptions is that you can reduce the number of error checks. Currently we have an error-checking structure roughly like this:

I know you can do this.

But it is a programming style that I hate.

It makes debugging and groking the error handling in a complex application harder rather than simpler.

I don't see a strong benifit for WordPress in switching to exceptions over handling errors correctly where they occur.

I never want a theme author to have to know what an Exception even is!

[scribu]

I agree with westi. The only place exceptions would be somewhat useful would be in the taxonomy API, but since most of the functions there can also be called directly, you would probably end up with more try / catch blocks than is_wp_error() checks.

[hakre]

It's interesting to see which opinions get exchanged here but I don't think it's possible to smash that easily the one against the other. The ​overall topic of Error and Exception Handling is far less than trivial.

So thanks for sharing your opinions and participating in this ticket.

Mine is quite simple: Switching to Exceptions for error handling in PHP makes code more clean, easier to test, simpler and needs less code. It enables one to write more complex applications. Debugging benefits highly from using exceptions.

Next to that exceptions can be used to signal more than only error exceptions.

But the question was: does it make sense for Wordpress? For those who fear change, most certainly not, they would highly dislike to adopt. Introducing something with no acceptance is a no-go. For those who think that it introduces more work, the acceptance will be comparable low.

As this ticket has already seen it's active period I will close it as fixed as my original question is answered and extensive feedback has been given:

No, there is no actual need of an exception class that is part of WordPress seen.

Feel free to re-open.

[hakre]

Replying to scribu:

.. The only place exceptions would be somewhat useful would be in the taxonomy API, but since most of the functions there can also be called directly, you would probably end up with more try / catch blocks than is_wp_error() checks.

I do not know that much about the taxnonomy API, but a place where returning error values does not work is the instantiation of objects. Constructors can never return a WP_Error object nor false on error (as long as the class of the object is not WP_Error).

[dd32]

As no change has been made to WordPress here, and the Fixed status is reserved for an issue being fixed (either in core, or beign dealt with externally on WordPress.org or similar); I'm going to close this as wontfix, this leaves for the potential for revisiting someday.