Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#34551 closed enhancement (fixed)

Change API error response to focus on singular errors

Reported by: rmccue's profile rmccue Owned by: johnbillion's profile johnbillion
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

(Migrating https://github.com/WP-API/WP-API/issues/1473 here.)

Basically: right now, the API returns a list of objects on error:

[
    {
        "code": "rest_no_route",
        "message": "No route was found matching the URL and request method",
        "data": {
            "status": ​404
        }
    }
]

This is because under the hood, WP_Error holds multiple errors. Most people don't use this functionality at all, and this complicates error handling on the client site.

This is p late, but still early enough to change this IMO. Needs sign-off though, since it's technically a BC break, and it cuts out some potential use cases of WP_Error

Attachments (2)

34551.diff (1.1 KB) - added by rmccue 9 years ago.
Return only a single error
34551.1.diff (2.2 KB) - added by danielbachhuber 9 years ago.
Handle new error format in WP_REST_Response::as_error()

Download all attachments as: .zip

Change History (14)

#1 follow-up: @dd32
9 years ago

Since this is a BC break, if we're going to do it, it has to happen now obviously.

I'd almost expect it to return the "primary" error object, with any additional errors available in an extra sub-key

An example use-case for where multiple errors are returned would significantly help in deciding if we do need to retain this though.

@rmccue
9 years ago

Return only a single error

#2 in reply to: ↑ 1 @rmccue
9 years ago

  • Keywords has-patch added

Replying to dd32:

I'd almost expect it to return the "primary" error object, with any additional errors available in an extra sub-key

Agreed, it's a bit non-obvious there.

An example use-case for where multiple errors are returned would significantly help in deciding if we do need to retain this though.

There's a few potential cases for this. One of the key ones might be multiple validation errors, so e.g. you're trying to create a post, but a) you're not authorised, and b) you sent an invalid post date.

Attached 34551.diff as an initial patch at this.

This ticket was mentioned in Slack in #core-restapi by rmccue. View the logs.


9 years ago

#4 @wonderboymusic
9 years ago

  • Owner set to rmccue
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-restapi by rmccue. View the logs.


9 years ago

#6 @rmccue
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 35653:

REST API: Optimise for singular error instances.

Previously, the API returned a list of errors, as WP_Error can hold multiple
error codes internally. This isn't a particularly common use case, and it
makes handling errors on the client side more complex than it needs to be.

Fixes #34551.

#7 @rmccue
9 years ago

In 35654:

REST API: Update tests for [35653]

See #34551.

#8 @danielbachhuber
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

WP_REST_Response::as_error() needs an update.

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


9 years ago

@danielbachhuber
9 years ago

Handle new error format in WP_REST_Response::as_error()

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


9 years ago

#11 @johnbillion
9 years ago

  • Keywords has-unit-tests added; dev-feedback removed
  • Owner changed from rmccue to johnbillion
  • Status changed from reopened to assigned

#12 @johnbillion
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 35671:

Update WP_REST_Response::as_error() to handle the new format error responses introduced in [35653].

Props danielbachhuber
Fixes #34551

Note: See TracTickets for help on using tickets.