Opened 9 years ago
Last modified 8 years ago
#35425 new enhancement
Return HTTP status code in WP_Error objects
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | REST API | Keywords: | needs-patch |
Focuses: | Cc: |
Description
The REST API has an existing pattern of using a HTTP status code to better describe the nature of a WP_Error
object:
if ( ! empty( $request['id'] ) ) { return new WP_Error( 'rest_post_exists', __( 'Cannot create existing post.' ), array( 'status' => 400 ) ); }
Throughout WordPress, it would improve the quality of error reporting to include an equivalent HTTP status when generating a new WP_Error
object.
This ticket can serve as placeholder for improvements to this effect.
Previously: https://github.com/WP-API/WP-API/issues/153#issuecomment-170734687
Attachments (2)
Change History (13)
This ticket was mentioned in Slack in #core by joehoyle. View the logs.
9 years ago
#4
@
9 years ago
Feels like cruft to me, but if that's the direction its headed I can update the patch.
#7
follow-up:
↓ 8
@
9 years ago
I don't think we can change the data already being passed in WP_Error
objects, as this'll break compatibility. Also, we can skip specifying WP_HTTP::INTERNAL_SERVER_ERROR
explicitly, just falling back to the default should be fine.
#8
in reply to:
↑ 7
;
follow-up:
↓ 10
@
9 years ago
Replying to rmccue:
I don't think we can change the data already being passed in
WP_Error
objects, as this'll break compatibility.
Can you clarify what the alternative should be? I thought changing the data included in WP_Error
objects was the original intent.
#9
@
9 years ago
Altering the data returned by existing WP_Error
s is going to be a no-go.
Potentially it could be "upgraded" to accept array( 'status' => 404, 'data' => ... )
and return the data element as the get_error_data()
and status via get_error_type()
/ get_error_status()
.
This will need some clear cut explanations of what HTTP code represents what as well, for example, a 404
being returned on download failure (as in the patch above) is the closest to what was actually encountered, but probably isn't what should be returned to the Browser or REST client (but it might be?).
#10
in reply to:
↑ 8
@
9 years ago
Replying to danielbachhuber:
Can you clarify what the alternative should be? I thought changing the data included in
WP_Error
objects was the original intent.
If the error object already has data, we can't change that (unless it's an assoc array we can just add a key to), since plugins may be relying on accessing that data as-is. For errors without data attached, we can add it, since plugins can't be relying on something that's not there. (Technically they could be relying on it being empty, but I doubt anyone ever does that.)
Replying to dd32:
Potentially it could be "upgraded" to accept
array( 'status' => 404, 'data' => ... )
and return the data element as theget_error_data()
and status viaget_error_type()
/get_error_status()
.
I think at that point, we'd be better off just adding a separate status
property. Incidentally, I'm fine with that, but we'd need to keep BC for having it in data anyway, so I don't think it's worth the cruft.
35425.wp-admin.diff updates all returned WP_Errors in the wp-admin dir.