[wp-hackers] Why WP_Error Sucks

[Ryan McCue]

Andrew Nacin wrote:
> You also mention the idea of putting try/catch inside of the plugin API —
> that would be terribly slow, and defeats the purpose of forcing exceptions
> on developers. Congratulations, a plugin developer didn't code for an
> exception because they knew core would catch it and issue a wp_die()
> message, like that error would ever happen anyway. No thanks.

To clarify what I said in the blog post: the idea of putting them inside
do_action/apply_filters would be to avoid calling wp_die() and instead
letting just that callback fail.

At the moment, a lot of developers forget about WP_Error (at least, in
the plugin audits I've done). This means that it actually causes a
warning (attempting to access object as array, e.g.) or worse, gives bad
data.

> 1. As return codes. Often, WP_Error is used when the error is not
> "exceptional." It enables you to pass multiple errors, and also extra data.

Definitely agree. There are plenty of places where it does make sense to
use WP_Error instead of an exception. I stereotyped all errors in my
blog post to be the type suiting exceptions, because there are a lot
that are. For those that aren't, WP_Error should be used instead,
although in some cases, a more specific class might be more useful.

As for the multiple errors/extra data, both of those are possible with
exceptions. Exceptions are still normal objects.


> 2. As legitimate errors where exceptions could be used, but shouldn't be.
> This kind of factors into return codes. I personally do not find a failed
> HTTP request to be "exceptional" that, if uncaught, should result in a
> fatal error.

I also agree here, but only for certain cases:

- cURL/fsockopen/Stream errors should be exceptions. These are things
  which should be raised, as they are a failure regardless of use case

- HTTP errors (i.e. non-2xx status codes) should not be exceptions,
  unless the caller specifically asks for them to be. For example,
  Requests [0] has a `throw_for_error()` method on the response object;
  Mike suggested an 'exception' option, which would also work great.

  The reason for this is because it's dependent on the caller as to
  whether one of these statuses is a failure mode.

> Wrapping all of this in a try/catch would be nice. (I think relying on
> exceptions to bubble is lazy; we'd be handling these all within that
> routine.)

It's less about being lazy and more about handling it where it makes
sense, rather than having to handle it at every stage.

> 
> However, two major problems with this. One,
> wp-admin/includes/update-core.php is copied over during a core update and
> executed by the currently installed version. Can't throw a WP_Exception
> when WP_Exception doesn't exist yet.

Why not include a mock WP_Exception within that? We already have mock
functions in wp-admin/load-scripts.php and wp-admin/load-styles.php.

On top of that, we could just use Exception itself. The functions that
have it available could use WP_Exception, and the other functions could
use Exception; the calling code could then catch all of them with `catch
(Exception $e)`

> The second problem is that PHP doesn't have finally blocks.

I do agree with that, and I'm not a fan of that, however they are
working on that (it looks likely that it'll make it into 5.5, and
possibly 5.4). That doesn't make much of a difference to us, but you can
fake it in the few cases where you actually need it:

	try {
		$fp = fopen('readme.txt');
		some_function_that_throws_an_exception($fp);
		fclose($fp);
	}
	catch (Exception $e) {
		fclose($fp);
		throw $e;
	}

It does involve duplication, but it's still possible, and I think it's
used infrequently enough that it's not too much of an issue.

> I'm not saying there isn't a single use in WordPress for a WP_Exception. I
> know for a fact there are some, but I also don't think there are enough
> examples where an exception would be oh-just-so-much-better than error
> objects to justify a paradigm change. As it is, WordPress is getting
> complicated (almost too complicated) in certain areas, and it'll only serve
> to hurt theme developers, designers, and many weaker plugin authors the
> more "by the book" we become.

WordPress is well-known throughout a lot of the PHP community for being
fairly horrible from a PHP standpoint. This has been changing as WP gets
modernised and I think part of that is embracing the facilities we have
in PHP.

I also think this will benefit plugin authors, especially the weaker
ones. A lot of them don't bother to check `is_wp_error()`. Exceptions on
the other hand make it very obvious when an error has occurred, and are
much less obscure than a "cannot use object of type WP_Error as array"
error.

Theme developers/designers should never need to handle that. Exceptions
don't make sense for any of the template tags (e.g.), only the more
low-level stuff.

> And of course, plugin authors can already use exceptions if they wanted to.

And they should, hence why I wrote about converting WP_Error to
exceptions now. I already do this myself. :)

> Write your own HTTP API wrapper that throws exceptions — come on, you're a
> programmer, do what programmers do: make a personal, convoluted abstraction
> layer. :-)

Actually, my plan was to write a wrapper for Requests [0] to make it
expose a WP_Http-like API, including WP_Error support (so basically, in
the other direction). Convolution FTW! :D

Thanks for the detailed response!


[0]: http://requests.ryanmccue.info/

-- 
Ryan McCue
<http://ryanmccue.info/>