#25155 closed defect (bug) (wontfix)
All functions returning string|WP_Error -- there is no WP_Error::__toString
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | minor | Version: | 2.1 |
Component: | General | Keywords: | |
Focuses: | Cc: |
Description
Example from class-wp-terms-list-table.php
:
$actions['view'] = '<a href="' . get_term_link( $tag ) . '">' . __( 'View' ) . '</a>';
What happens if get_term_link
returns WP_Error
, in this case?
a) That part of the code, and all similar places should be fixed, I guess - to check for error before printing.
b) A WP_Error::__toString
method might help in some cases. It may return an empty string, or some string constant, or WP_Error::get_error_message
Change History (4)
#1
in reply to:
↑ description
;
follow-up:
↓ 2
@
12 years ago
#2
in reply to:
↑ 1
;
follow-up:
↓ 3
@
12 years ago
Replying to nacin:
In this case, we know the term exists.
That's great, as long as the same people, who remember every bit of the core code, remain on the project.... A safe approach would be to check anyway.
I'm not sure if
__toString()
for WP_Error is a good idea. Right now, failing to catch a WP_Error often leads to a fatal error — which is often good, rather than bad, as it alerts developers to problems as they write their code. But more so, as you state — I'm not sure if it should be get_error_message(), or an empty string.
Yes, but still, a function cannot return string|Something, if Something cannot be converted to a string. That's a major "boo-boo" in my opinion, and any PHP code validator should explode with warnings about that. (At least my IDE complains quite loudly).
I remember days when errors were raised, but not returned as objects ;-)
As for the fatal errors, they usually happen on production... when you are on vacation in a desert, with no internet connection.
#3
in reply to:
↑ 2
@
12 years ago
- Resolution set to wontfix
- Status changed from new to closed
- Version set to 2.1
Replying to tivnet:
Replying to nacin:
In this case, we know the term exists.
That's great, as long as the same people, who remember every bit of the core code, remain on the project.... A safe approach would be to check anyway.
In this case WE KNOW the term exists because it is in core and if the term didn't exist you would get a notice for trying to get property of non object and a `Catchable fatal error: Object of class WP_Error could not be converted to string message inside your list table (with WP_DEBUG set to false). If the term doesn't exist when calling that line you have much bigger problems than an array to string error.
When using functions in your own code that could return WP_Error you get to decide how to handle it. You could ignore it by doing:
if ( ! is_wp_error( $thingy ) ) { //do stuff
or you could convert it to a string and return it.
We shouldn't be supporting bad coding habits in core. Closing as wontfix.
Replying to tivnet:
get_term_link() only returns WP_Error if the term doesn't exist. In this case, we know the term exists.
I'm not sure if
__toString()
for WP_Error is a good idea. Right now, failing to catch a WP_Error often leads to a fatal error — which is often good, rather than bad, as it alerts developers to problems as they write their code. But more so, as you state — I'm not sure if it should be get_error_message(), or an empty string.