985919 - Loop Server — Errors should be returned using a defined format

Status: VERIFIED FIXED

(Hello (Loop) :: Server, defect)

Description

[Alexis Metaireau (:alexis)]

It seems to me it would be easier for the client to have a defined protocol for the errors.

For the token server, we decided to use the cornice format for the errors, and that seemed to be useful.

Basically, the top-level JSON object in the response will always contain a key named "status", which maps to a string identifying the cause of the error. Unexpected errors will have a status string of “error”; errors expected as part of the protocol flow will have a specific status.

Errors will have three different keys:

- location is the location of the error. It can be “querystring”, “header” or “body”
- name is the eventual name of the value that caused problems
- description is a description of the problem encountered.

For instance, that would be something like:

{
  "status": "errors",
  "errors": [{"location": "body",
              "name": "version",
              "description": "version should be an integer"
             }]
}

Nico, Dmose, Standard8, do you think that would make sense to use a similar paradigm for the loop server?

Comment 1

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

LGTM

Comment 2

[Mark Banner (:standard8)]

That should be fine, as long as we don't need to localise the descriptions, then it'll be good.

Comment 3

[Rémy Hubscher (:natim)]

Attachment: Link to GitHub PR.

Comment 4

[Alexis Metaireau (:alexis)]

Comment on attachment 8395651 [details] [review]
Link to GitHub PR.

After discussions, makes more sense to implement this as a middleware that we can reuse easily, and that exposes an `addError` on the response object.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Re the format, I don't understand how the value of "name" is "version".  The structure suggested looks reasonable to me at first glance, though I suspect we won't really have useful opinions until we've used it for a little while.

Comment 6

[Rémy Hubscher (:natim)]

I had a similar interogation.

Actually, the format is as follow for a 400 errors.

 - location is the location where the parameter has been looked for (url, querystring, body)
 - name is the variable name (version, token, callerId, user, etc)
 - description is the human readable message.

This helps the client developer (Server user) to understand what is missing (Document Oriented Error Messages)

Comment 7

[Rémy Hubscher (:natim)]

Comment on attachment 8395651 [details] [review]
Link to GitHub PR.

Updated the PR with the middleware.

Comment 8

[Alexis Metaireau (:alexis)]

Comment on attachment 8395651 [details] [review]
Link to GitHub PR.

That's far better but still missing some bits. Declining for now, but we're almost there :)

Comment 9

[Rémy Hubscher (:natim)]

Comment on attachment 8395651 [details] [review]
Link to GitHub PR.

Fixing comments.

Comment 10

[Alexis Metaireau (:alexis)]

I directly addressed some of my comments and merged it, thanks!

https://github.com/mozilla-services/loop-server/commit/aa0a7ba4d450a814a521631bdbdec5da3de4246f

Comment 11

[James Bonacci [:jbonacci]]

Closing out the older bugs. The current code is newer than this commit.