Laravel-json-api: Attach meta to error objects

[gendronb]

Hi,

First of all, thanks for your work on this awesome project..

I'm current;y investigating the possibility of adding meta information to the attributes validation errors returned when creating/updating resources (as per JSON-API v1.0 specs, https://jsonapi.org/format/#error-objects).

My use case would be to add a machine-readable description of each validation error, to complement the human-readable one found under "details"... Something like :

{
    "status": "422",
    "title": "Unprocessable Entity",
    "detail": "The status must be between 1 and 5.",
    "source": {
        "pointer": "/data/attributes/status"
    },
    "meta": {
        "machine_description": {
            "between": [
                "1",
                "5"
            ]
        }
    }
}

With Laravel, calling $validator->failed() returns a pretty decent (when JSON-encoded) machine-readable description of all errors, for example, given the following rules :

[
 'title' => 'bail|required|string|min:5|max:10',
 'pages' => 'bail|required|numeric',
 'body' => 'required|string|same:title|not_in:foo,bar',
 'else' => 'required',
 'comments' => 'bail|min:3',
 'comments.*.author' => 'bail|required|string',
]

and the following attributes :

{
    "title": "foo",
    "pages": "a",
    "body": "foo",
    "else": "test"
}

$validator->failed() would return something like this :

{
    "title": {
        "min": [
            "5"
        ]
    },
    "pages": {
        "numeric": []
    },
    "body": {
        "notin": [
            "foo",
            "bar"
        ]
    }
}

Anyway... the actual use of this feature is not important here, my question is simply : it is currently possible to add meta info to error objects? If not, where should I begin to investigate in order to achieve such a thing?

Maybe a new method (getMeta) could be added to the AbtractValidators class, which would access the validator instance and the error path (source/pointer), the return value being added as meta to the error?

Thanks for your help...

[lindyhopchris]

Ah I did not realise Laravel returned that information in the JSON. I think it would be good for this package to automatically add that data to the error meta. So I'll add this to the feature list but it'll probably not land until the after the 1.0.0 release.

[gendronb]

@lindyhopchris

Great to here that, but should this be by default?

Maybe someone else's use case would be to return something else?

I was only looking for a way to hook into the error's response generation, but wasn't sure where to look at in the codebase...

As I said in my first comment, if there was a method somewhere that could be overridden (when generating each attribute error), that would be passed the validator instance and probably the path of the error (in order to retrieve the relevant part of the validation error), one could use that method to return whatever data he wants for his particuler use case in the meta tag...

My 2 cents...

[lindyhopchris]

Good point. I think it should potentially be the default because if Laravel does it by default in the JSON for validation messages, then this package should replicate what Laravel does in JSON but in the JSON API format.

Definitely needs more research. In terms of overriding the implementation, but the current class that is generating the errors from the validations isn't given access to the data that you'd need. I'll have to look into this in a bit more detail to see how it could be implemented.

[gendronb]

@lindyhopchris

Just to mention, by default, Laravel doesn't return the "machine-readable" representation, even in a JSON reponse. It returns the localized error message :

"email": [
            "The email has already been taken."
]

That's why I think this should NOT be the default...

But the structured information returned by the "failed()" method could really be useful when building a stateless API...

[lindyhopchris]

Ah ok I'd misunderstood! I'll take a look as to how you could implement this. Actually we have a production API where it would be really useful to include this information, so I can definitely see the benefit of supporting this.

[lindyhopchris]

@gendronb

Have investigated this and it definitely looks possible. The only thing is that the info from $validator->failed() doesn't match up with the error messages. So you can't tell which rule in the failed array caused the message that results in the error object detail member. This is a problem if you've got multiple validation messages for the same attribute.

There's two potential solutions for this, either:

1. Include the failed info in the top-level meta, not the error meta.
2. Include the failed info in the error object meta, even if there are multiple messages for the same attribute. I.e. each error object has the failed details in it for that pointer, but if there are multiple errors for the same pointer then both errors would have the same failed data in it.

Examples of the two approaches if two messages for the same pointer are below.

What do you think? I prefer 2 - i.e. include it in the error object. Of course developers can always write their rules to bail, then they could guarantee that there is only one failure rule per message.

Approach 1: Top-Level Meta

{
  "meta": {
    "failed": [
      {
        "source": {
          "pointer": "/data/attributes/number"
        },
        "rules": {
          "between": [
            "1",
            "4"
          ],
          "in": [
            "2",
            "4"
          ]
        }
      }
    ]
  },
  "errors": [
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The number must be between 1 and 5.",
      "source": {
        "pointer": "/data/attributes/number"
      }
    },
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The number must be even.",
      "source": {
        "pointer": "/data/attributes/number"
      }
    }
  ]
}

Approach 2: Error Meta

{
  "errors": [
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The number must be between 1 and 5.",
      "source": {
        "pointer": "/data/attributes/number"
      },
      "meta": {
        "failed": {
          "between": [
            "1",
            "4"
          ],
          "in": [
            "2",
            "4"
          ]
        }
      }
    },
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The number must be even.",
      "source": {
        "pointer": "/data/attributes/number"
      },
      "meta": {
        "failed": {
          "between": [
            "1",
            "4"
          ],
          "in": [
            "2",
            "4"
          ]
        }
      }
    }
  ]
}

[lindyhopchris]

One thing I need to check is whether the order the messages and the failed data match up... in which case I could potentially match the two up. It feels a bit unsafe though because I'm totally reliant on the validator holding them in the same order, which I'm not in control of...!

[gendronb]

@lindyhopchris

Thanks for your reply.

Hmmm... I also prefer option 2, but only if validation errors are not duplicated. Option 1 would be really simple, but feels a little awkward...

This said, if the order of messages and failed data really line up everytime, great, go for it, but I'm with you that it feels pretty unsafe.

Could you point me to the location in code where all this magic occurs ;-) ?

[lindyhopchris]

@gendronb

I've just pushed a issue263 branch with a single commit that adds in the failed meta to each error object - so you can see on that how it's wired in. The main thing is the ErrorTranslator is where the error object is created. Although not yet documented, overriding the ErrorTranslator in the service container is how I'd intend an application to customise the errors that are created. The other changes on the commit are about ensuring that the error translator is given the failed information in addition to the validation key and detail.

The order of the messages and failed data works in Laravel 5.5, 5.6 and 5.7. It does feel a little unsafe but I think this is a fantastic feature so I'm inclined to add it for 1.0.0 as an opt-in feature. (I'd have to add some code as to how to opt in to it.)

Would be good if you could have a play with the issue263 branch in your application and let me know how you get on. If it meets your requirements, I'll add the opt-in code then merge.

[gendronb]

I'll give it a try next week when I have a few minutes, stay tuned!

Thanks again!

[gendronb]

@lindyhopchris

When using the issue263 branch, for any requests (that were working fine before, same headers), I get the following errors :

    { "errors": [ { "status": "406", "title": "Not Acceptable", "detail": "The requested resource is capable of generating only content not acceptable according to the Accept headers sent in the request." } ] }    

Did I miss something?

[lindyhopchris]

Ah yeah, good point - should have mentioned that!

You'll need to follow the 1.0.0-beta.6 to 1.0.0-rc.1 upgrade notes that are here:
https://github.com/cloudcreativity/laravel-json-api/blob/develop/docs/upgrade.md

Basically you're getting that message because your API's config needs to be updated.

[gendronb]

@lindyhopchris

Thanks for the update!

Ok, now it's working properly, and it's really awesome!

I see you added an "options" key for rules "with options" or parameters: don't you think this "options" key should be removed if no options are present. For exemple, for a "numeric" rule, we currently get the following response :

       "failed": {
          "rule": "numeric",
          "options": []
        }

the "options" is really superfluous here...

Also, I have a small doubt about the naming of the "failed" key... "validation_failed" maybe?

Thanks!

[gendronb]

Also, given the following rules:

[
   "status" => "required|string|in:closed,open",
   "height" => "string|min:0|max:10",
   "width" => "required_with:height"
]

and the following input (notice the "height" is an integer):

{
  "data": {
    "type": "events",
    "attributes": {
        "status": "open",
        "height": 10
    }
  }
}

we get this response (no meta > failed key for the "width" field) :

{
  "errors": [
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The height must be a string.",
      "source": {
        "pointer": "\/data\/attributes\/height"
      },
      "meta": {
        "failed": {
          "rule": "string",
          "options": []
        }
      }
    },
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The width field is required when height is present.",
      "source": {
        "pointer": "\/data"
      }
    }
  ]
}

If I change the "height" rule to be numeric, the response is then correct (see meta > failed key for field "width") :

{
  "errors": [
    {
      "status": "422",
      "title": "Unprocessable Entity",
      "detail": "The width field is required when height is present.",
      "source": {
        "pointer": "\/data"
      },
      "meta": {
        "failed": {
          "rule": "required-with",
          "options": [
            "height"
          ]
        }
      }
    }
  ]
}

[lindyhopchris]

I've pushed a commit to that branch that gets rid of options if there are none.

I'm happy to rename the failed meta key... however would prefer it to be a single word. The moment I use something like validation_failed I need to know whether the API is using dash-case, underscored or camel-cased member names.... which overly complicates it.

For your height/width thing, that would seem to me to be a Laravel thing, as the package can only work with the info provided to it from the Validator::failed() method. Maybe dd the failed array that Laravel is giving and see what the difference is in the two scenarios you've highlighted?

[gendronb]

So let's leave the "failed" meta key as it is... You're right, having a single word is simpler, and I don't have any alternative in mind. Maybe this could eventually be user-configurable?

As fo the height/width problem, I understand and will further investigate and keep you informed...

[lindyhopchris]

@gendronb

So I'm going to add the code to opt-in to this, then merge it.

The one thing I'd say having played around with it the concern I have is the amount of backend implementation detail it reveals to a client. For example, the options for the exists rule reveals information about your database implementation. For a rule object, it reveals the fully qualified PHP class name of the rule.

Personally I wouldn't put that kind of information in an API, as one of the purposes of an API is to separate clients from the actual server implementation (it's a translation layer). Would be interested to hear what you think about that kind of information being revealed to a client?

[gendronb]

@lindyhopchris

Depends on the rule... I didn't make exhaustive testing of all the available rules, but I think most rules don't leak much critical info to the user (think min, max, between, etc).

Maybe should we treat some rules differently (as you mentionned, "exists" and "rule objects"), and "sanitize" their ouput, but I have to confess I didn't have time to think about it...

[lindyhopchris]

So my plan is to santise the database ones and also to run the rule name through the translator so that it can be translated if desired. That means that someone could add translations for FQNs of rule objects.

Hoping to get this done sometime next week - been on frontend stuff this week so not been able to sort this out yet.

[gendronb]

Awesome @lindyhopchris, will use this very soon in our current project. Thanks again!