Opened 3 years ago
Last modified 4 months ago
#57842 new enhancement
Add a new independent WP_Exception class, compatible with WP_Error
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | General | Keywords: | has-patch |
| Focuses: | Cc: |
Description
Exceptions offer a better way of working with errors. Whilst we all appreciate WP_Error and how deeply integrated into WordPress it is, we should at least start to think about introducing an independent and official WP_Exception class.
I don't think I will need to elaborate the benefits Exceptions bring into modern development on this ticket.
Instead, perhaps we could use this ticket to discuss a way to officially introduce an Exception as a starting point.
Once this is accepted, we could then discuss the pros and cons of updating existing code to use this new exception in other tickets. But we should, at the very least, offer a standard Exception for developers to start implementing in their custom code bases and plugins.
I've created a super simple WP_Exception class which is compatible with WP_Error as a starting point. Happy to have a serious discussion on how best to improve this and get something like this merged into core.
Sample usage:
// Normal Exception.
try {
throw new WP_Exception( 'error_code', 'Error message', $error_data );
} catch ( WP_Exception $e ) {
$e->get_error_code();
$e->get_error_message();
$e->get_error_data();
}
// To WP Error.
try {
my_function();
} catch ( WP_Exception $e ) {
return $e->to_wp_error(); // Returns a WP_Error based on the exception.
}
// From WP Error.
if ( is_wp_error( $thing ) ) {
$exception = new WP_Exception();
throw $exception->from_wp_error( $thing );
}
Attachments (1)
Change History (11)
This ticket was mentioned in PR #4159 on WordPress/wordpress-develop by junaidbhura.
3 years ago
#1
#2
follow-up:
↓ 6
@
3 years ago
That could be a great improvement. We need to modernize things that can be modernized into the core. I think this is a great way to start.
By doing it, we can use agnostics errors handlers (for example).
Thanks for your work 👌
#4
@
4 months ago
@junaidbhura Sorry, I missed this at the time you opened it.
I like that the POC includes an interaction with WP_Error but it would be good to go through an architecture phase before implimenting.
#5
@
4 months ago
Thanks @peterwilsoncc! Could you guide me on how we could begin an architecture phase for this?
#6
in reply to:
↑ 2
@
4 months ago
Replying to benjgrolleau:
That could be a great improvement. We need to modernize things that can be modernized into the core. I think this is a great way to start.
By doing it, we can use agnostics errors handlers (for example).
Thanks for your work 👌
Thanks @benjgrolleau! Hope we can get some momentum behind this!
#7
@
4 months ago
WP_Error has its place, but having a proper WP_Exception in core would be a big win for modern development. Even a basic version gives devs a cleaner, more standard way to handle errors. Definitely support getting this in as a starting point.
#8
follow-up:
↓ 9
@
4 months ago
Thanks @junaidbhura for proposing this.
First of all, the patch itself seems rather harmless. You covered what was also my first thought — interaction with WP_Error.
As per the architectural part though, I think it would be useful to motivate this change more.
I don't think I will need to elaborate the benefits Exceptions bring into modern development on this ticket.
You wrote this in the description, but I think it might reveal some assumptions you could be bringing into the discussion. For one, exception-based errors are already possible in WordPress code. Yes, generally they are avoided to keep things running even when misbehaving plugins run. However, this change hints at a move away from WP_Error, yet intentionally doesn’t expand on that.
I think it would be helpful to expand on that.
In addition, maybe it would help to engage with a few basic questions I have:
- Why is this preferred to
ValueError,TypeError,Error,Exception? In other words, if the proposal is to push WordPress code towards Exceptions, what value does a custom exception bring to the table? Can you share some code examples that highlight it?
- What is the functional goal of transforming into and out of
WP_Error? Is it to provide a smooth transition, where a function throws and then a calling function returns the transformed value?
- Did you consider adjusting
WP_Errorto extendExceptionitself rather than creating a newWP_Exception? What would it look like in comparison if we could simply return or throw aWP_Error?
#9
in reply to:
↑ 8
@
4 months ago
Replying to dmsnell:
- Did you consider adjusting
WP_Errorto extendExceptionitself rather than creating a newWP_Exception? What would it look like in comparison if we could simply return or throw aWP_Error?
I had this thought too, unfortunately the constructor isn't compatible with Throwable.
#10
@
4 months ago
Thanks for your response @dmsnell and @johnbillion !
Thanks for pointing out that I should probably elaborate what I mean, instead of assuming what I write will make sense to everyone!
I should make this clarification: I am NOT proposing to replace WP_Error. It has its place, and has very deep integration with WordPress core.
What I am proposing is to create a NEW WP_Exception class - which will be an official WordPress exception, which developers can start using, which will help aid in working with WP_Error
@dmsnell Allow me to respond to your questions:
---
Why is this preferred to ValueError, TypeError, Error, Exception? In other words, if the proposal is to push WordPress code towards Exceptions, what value does a custom exception bring to the table? Can you share some code examples that highlight it?
The proposal is not to push WordPress core towards Exceptions, but rather to provide an additional way for developers to handle errors / exceptions. I will be adding some sample code after these questions to elaborate this.
---
What is the functional goal of transforming into and out of WP_Error? Is it to provide a smooth transition, where a function throws and then a calling function returns the transformed value?
Again, the proposal is not to replace WP_Error. Since WordPress core is reliant on WP_Error, and easy to use Exception to convert it into a WP_Error will be beneficial. We can consider firing a hook for example do_action( 'wp_exception', $this ) every time an exception is triggered, which can be beneficial for logging, etc. Or anything else that we may require in the future, it keeps things scalable, and will provide an "official" way for developers to throw exceptions. Example class MyException extends WP_Exception. Right now different plugin vendors use their own Exception classes.
---
Did you consider adjusting WP_Error to extend Exception itself rather than creating a new WP_Exception? What would it look like in comparison if we could simply return or throw a WP_Error?
This may not be a bad idea, and something we can explore.
---
Let me elaborate a use case of using WP_Exception here:
Consider the following example, where a function returns a WP_Error which needs to be used by a REST API endpoint:
<?php // Without WP_Exception. function rest_api_handler(): array|WP_Error { $value_1 = heavy_sub_function_1(); if ( $value_1 instanceof WP_Error ) { do_action( 'my_custom_action' ); return $value_1; } $value_2 = heavy_sub_function_2(); if ( $value_2 instanceof WP_Error ) { do_action( 'my_custom_action' ); return $value_2; } $value_3 = heavy_sub_function_3(); if ( $value_3 instanceof WP_Error ) { do_action( 'my_custom_action' ); return $value_3; } if ( $some_risky_action ) { do_action( 'my_custom_action' ); return new WP_Error( 'error_code', $data ); } return $value; }
<?php // With WP Exception. function rest_api_handler(): array|WP_Error { try { $value_1 = heavy_sub_function_1(); // Throws WP_Exception $value_2 = heavy_sub_function_2(); // Throws WP_Exception $value_3 = heavy_sub_function_3(); // Throws WP_Exception if ( $some_risky_action ) { throw new WP_Exception( 'error_code', $data ); } } catch ( WP_Exception $e ) { do_action( 'my_custom_action' ); return $e->to_wp_error(); } return $value; }
Trac ticket: https://core.trac.wordpress.org/ticket/57842