[wp-trac] [WordPress Trac] #46191: REST API parameter validation should handle multiple error messages/codes. (was: WP_REST_Request::has_valid_params() should utilize get_error_messages() and not get_error_message(). Unexpected behavior occurs when you try to WP_Error->add() in the REST API)

[WordPress Trac]

#46191: REST API parameter validation should handle multiple error messages/codes.
--------------------------------------+-----------------------
 Reporter:  tmfespresso               |       Owner:  (none)
     Type:  defect (bug)              |      Status:  new
 Priority:  normal                    |   Milestone:  5.8
Component:  REST API                  |     Version:  4.4
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  rest-api
--------------------------------------+-----------------------
Changes (by TimothyBlynJacobs):

 * milestone:  5.7 => 5.8


Comment:

 Thanks for testing @xkon! I'm reiterating some of our conversation on
 Slack to note it for posterity. And this sort of turned into a stream of
 consciousness, so apologies in advance :D.

 Yeah so one of the issues is that error data is attached to the error
 code, not the `code` and `message` together thru `add()`. Disambiguating
 this is I think essentially impossible because even if we did somehow make
 changes to `add` to keep track of this, a user can just call `add_data` at
 anytime.

 I was thinking we might be able to only include data for the first time an
 error code appears. So for example:


 {{{
 {
   "code": "rest_invalid_param",
   "message": "Invalid parameter(s): id",
   "data": {
     "status": 400,
     "params": {
       "id": "This is our first error. Well this is yet another error :D."
     }
   },
   "additional_errors": [
     {
       "code": "rest_test_error",
       "message": "This is our first error.",
       "data": {
         "param": "id"
       },
       "param": "id",
       "additional_data": [
         {
           "status": 420
         },
         {
           "status": 410
         }
       ]
     },
     {
       "code": "rest_test_error",
       "message": "Well this is yet another error :D.",
       "param": "id"
     }
   ]
 }
 }}}

 This, however, would be a breaking change. We would have to instead only
 omit the `additional_data`, and keep the `data` around.

 {{{
 {
   "code": "rest_invalid_param",
   "message": "Invalid parameter(s): id",
   "data": {
     "status": 400,
     "params": {
       "id": "This is our first error. Well this is yet another error :D."
     }
   },
   "additional_errors": [
     {
       "code": "rest_test_error",
       "message": "This is our first error.",
       "data": {
         "param": "id"
       },
       "param": "id",
       "additional_data": [
         {
           "status": 420
         },
         {
           "status": 410
         }
       ]
     },
     {
       "code": "rest_test_error",
       "message": "Well this is yet another error :D.",
       "data": {
         "param": "id"
       },
       "param": "id"
     }
   ]
 }
 }}}

 This reduces the response size. But may make it harder for clients that
 need to operate on the error data more complicated. However, I suppose any
 operation on that would be wrong since they wouldn't be able to use the
 right data.

 I'm not sure what the correct move is here. If I could see anyway to
 actually track error data to a code/message, I'd punt until we could
 figure that out. But I'm really not sure what that would look like.

 For example, imagine two `enum` parameters `enum_a` and `enum_b`. If we
 wanted to get the `enum` for each, we'd run into an issue disambiguating.

 {{{
 {
   "code": "rest_invalid_param",
   "message": "Invalid parameter(s): enum_a, enum_b",
   "data": {
     "status": 400,
     "params": {
       "enum_a": "enum_a is not one of a, b, c.",
       "enum_b": "enum_b is not one of d, e, f."
     }
   },
   "additional_errors": [
     {
       "code": "rest_not_in_enum",
       "message": "enum_a is not one of a, b, c.",
       "data": {
         "param": "enum_a"
       },
       "param": "enum_a",
       "additional_data": [
         {
           "enum": ["a", "b", "c"]
         },
         {
           "enum": ["d", "e", "f"]
         }
       ]
     },
     {
       "code": "rest_not_in_enum",
       "message": "enum_b is not one of d, e, f.",
       "data": {
         "param": "enum_b"
       },
       "param": "enum_b",
       "additional_data": [
         {
           "enum": ["a", "b", "c"]
         },
         {
           "enum": ["d", "e", "f"]
         }
       ]
     }
   ]
 }
 }}}

 Thinking on it more, it would actually be worse since the same thing would
 happen to the `param` data.

 {{{
 {
   "code": "rest_invalid_param",
   "message": "Invalid parameter(s): enum_a, enum_b",
   "data": {
     "status": 400,
     "params": {
       "enum_a": "enum_a is not one of a, b, c.",
       "enum_b": "enum_b is not one of d, e, f."
     }
   },
   "additional_errors": [
     {
       "code": "rest_not_in_enum",
       "message": "enum_a is not one of a, b, c.",
       "data": {
         "param": "enum_a"
       },
       "param": "enum_a",
       "additional_data": [
         {
           "param": "enum_b"
         },
         {
           "enum": ["a", "b", "c"]
         },
         {
           "enum": ["d", "e", "f"]
         }
       ]
     },
     {
       "code": "rest_not_in_enum",
       "message": "enum_b is not one of d, e, f.",
       "data": {
         "param": "enum_a"
       },
       "param": "enum_a",
       "additional_data": [
         {
           "param": "enum_b"
         },
         {
           "enum": ["a", "b", "c"]
         },
         {
           "enum": ["d", "e", "f"]
         }
       ]
     }
   ]
 }
 }}}

 I wonder if @dlh or @johnbillion have any thoughts on this. It feels like
 an inescapable problem that data isn't attached to a `code` ''and''
 `message`.

 Given all that, I'm going to punt this. I'm not certain what the right
 solution is, or if there is one, but this definitely needs improvement.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46191#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform