error design about epilogue

[mbroadst]

@johngiudici @dchester

[dchester]

I agree with the two points you mentioned: we should avoid exposing Sequelize errors directly; and we should have some high-level error string in a consistent place somewhere in the response.

In other projects I've had success with a standard error response format that has a top-level message and code, along with an optional list of errors, a list of objects each of which may have their own message, code, and field identifier.

For example, for a user sign-up attempt failing for multiple reasons, you might find a response like this:

HTTP/1.1 400

{
  "message": "new user validation failed",
  "code": "validation_failed",
  "errors": [
    {
      "message": "username is already taken",
      "code": "username_taken",
      "field": "username"
    }, {
      "message": "please supply a valid email address",
      "code": "invalid_email",
      "field": "email"
    }
  ]
}

So that's pretty similar to your second "more RESTful" example, which seems better to me. How's that strike you?

[johngiudici]

I agree with the "more RESTful" approach. I think wrapping the whole response in an error property would be unnecessary. One point I would like to add, though, is that these error should look as similar to the sequelize errors as possible. I think it's a good idea for us to massage the errors into a single, standardized form, but I don't think it would be good to come up with something wildly different from what sequelize already presents.

[mbroadst]

@dchester okay yeah that's how I was leaning. My only problem with your approach is where do we get the "code" values? I'm inclined to use the format you posted there sans "code" fields.

[mbroadst]

okay an initial go at this can be found in: 8616619. Let me know what you guys think (this is in the 0.4.0 branch btw)

[bradisbell]

Have you considered adding the status code in a status field (or similar) in the error object? This enables better support for JSON-P, but might be a non-issue these days thanks to better CORS support making JSON-P irrelevant. This isn't a need I have specifically, but one I've ran into... just throwing it out there for discussion.

[mbroadst]

@bradisbell we made the step forward to unify the error representations and that's as far as it's gone to this point. I agree that status codes are nice in large projects for e.g. translation concerns, but I'm not sure this is something that needs to be fixed in epilogue but rather sequelize itself. If we start making status codes for epilogue we're just continuing a chain that starts with sequelize (sequelize is string matching to generate its own errors, passing those on to us to string match and generate our own codes, all of this starting with a code from the database - but only on SOME databases heh, etc etc).

I think the solution is to introduce error codes to sequelize to unify a status code for e.g. unique constraints across database implementations. Once that work is done, we can simply include the code into our messages. I'm sure the sequelize guys would take the patch, they're a great bunch to work with :)