holidayextras / jsonapi-server

A config driven NodeJS framework implementing json:api and GraphQL
MIT License
488 stars 115 forks source link

Non-standard JSON API error objects used #231

Open gauthierm opened 7 years ago

gauthierm commented 7 years ago

When an error is returned it is sent similar to the following:

{
  "jsonapi": {
    "version": "1.0"
  },
  "links": {
    "self": "http://localhost:5005/ENDPOINT"
  },
  "errors": [
    {
      "status": "403",
      "code": "EFORBIDDEN",
      "title": "Param validation failed",
      "detail": [
        {
          "message": "\"FIELD\" is not allowed to be empty",
          "path": "FIELD",
          "type": "any.empty",
          "context": {
            "key": "FIELD"
          }
        }
      ]
    }
  ]
}

The JSON API Error spec says the detail key should be a human-readable string but the server is returning an array of objects.

theninj4 commented 7 years ago

The type of detail is very loosely implied: a human-readable explanation specific to this occurrence of the problem.

For validation errors, the best detail we can provide is the result of the Joi validation library, to aid engineers when building and consuming API's. If we change this behaviour, it needs to produce a message that is helpful for all variations of validation failure - something I'm not confident in doing.

I'm in no rush to change this behaviour. There's an implication that we're not 100% to-spec, however it's a trade-off that makes this project easier to work with and consume.

gauthierm commented 7 years ago

As a programmer, I like the extra detail provided by Joi's validation library. I created this issue because I am writing a client using the spec as a reference and it failed to parse the errors detail values returned by jsonapi-server.

It may be worthwhile contacting the spec authors and asking for clarification on the error details field (is it an array, an object, a string, any of the above?)

In my case, I've updated my client to work with error objects returned by this library and string values, but I wouldn't expect other JSON API clients to necessarily do the same based on the spec. This will likely cause incompatibilies around error handling with other libraries.

jamesplease commented 7 years ago

The type that detail is intended to be may be loosely implied by the specification, but any ambiguity is cleared up in the schema. If this library is not validating its responses with that schema, then you might want to consider it: it can be really helpful to make sure things like this don't slip through the cracks!

The solution to the particular example in this issue is straightforward, I think: map each validation error to its own JSON API error. The "description" of that error should be the "message" from Joi. Then, the rest of the content of the Joi error can live happily in meta.

In other words:

errors: [{
  "status": "403",
  "code": "EFORBIDDEN",
  "title": "Param validation failed",
  "detail": "\"FIELD\" is not allowed to be empty",
  "meta": {
    "path": "FIELD",
    "type": "any.empty",
    "context": {
      "key": "FIELD"
    }
 }
}]

This does lead to more data being sent over the wire, but it's:

gnapse commented 7 years ago

I am confused because in my experience with the sample app provided in this repo, not even the regular response I get is jsonapi-compliant. I don't get the objects with the attributes section and the relationships section, and I don't see the related objects described with just the type and id, and then the actual objects in the included area. I see a regular json where objects are just put in place, even repeated if they appear more than once in the graph.

For instance, I get something like this:

{
  "data": {
    "articles": [
      {
        "title": "NodeJS Best Practices",
        "tags": [
          { "name": "live" }
        ]
      }
    ]
  }
}

When if I expect jsonapi, I would expect something like this:

{
  "data": [
    {
      "id": "1",
      "type": "articles",
      "attributes": {
        "title": "NodeJS Best Practices",
      },
      "relationships": {
        "tags": {
          "data: [
            { "id": "2", "type": "tags" }
          ]
        }
      }
    ]
  },
  "included": [
    {
      "id": "2",
      "type": "tags",
      "attributes": { "name": "live" }
    }
  ]
}

Am I missing something here?