Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error responses in API #95

Open
organisciak opened this issue Mar 1, 2016 · 5 comments
Open

Error responses in API #95

organisciak opened this issue Mar 1, 2016 · 5 comments

Comments

@organisciak
Copy link
Member

The API needs some format for returning errors, rather than giving a traceback (in the case of exceptions) or zeros (in the case of missing terms). This can communicate better down the line to clients.

This is one of the most important needs right now and I'm willing to work on it, but @bmschmidt should have the final say in the error response policy.

I propose a very simple response, { error: String(), errorcode: int() }

We can number errors as we account for them. For example:

Query Error

  1. Malformed query
  2. Term not indexed
  3. Too many words in phrase
  4. facetName doesn't exist
  5. Database databaseName does not exist

Server Error (i.e. please contact admin)
6. Cannot access host
7. Memory tables need to be rebuilt
8. Unknown server error

@bmschmidt
Copy link
Member

Good.

One issue with this format is that it is not completely obvious how to distinguish a failed call from a successful one. (A legimate bookworm response could, in theory, be a dict with the keys error and errorcode).

Would it be too cumbersome on the client-API side to return a 500 (Internal error), and then have the data portion of the error description contain the JSON format you propose?

@bmschmidt
Copy link
Member

One problem with my suggestion is that there are cases where the API is deployed locally rather than over HTTP.

Maybe an error should just be a single string. That would be unambiguous, because valid responses are always dicts or lists.

"ERROR 101: Query does not parse to valid JSON"
"ERROR 102: Search term 'word':'%s' not in database" <- This one will need to specify which term, exactly, isn't, and allow errors on other terms as well.
"ERROR 201: Cannot access host"
"ERROR 202: Database tables are empty"

@organisciak
Copy link
Member Author

My preference is to communicate in JSON, because that's what clients are looking for when they make the call, and it is what I usually see elsewhere. Your first suggestion sounds better for this reason, and because the header is what tells the callback for d3.json or $.json that there was an error.

Still, this brings up the issue that the existing response isn't structured in any formal way. It would be nice if it was wrapped in the way the very simple JSend spec suggests: {status: 'STRING', data: { ...BW-RESPONSE... }. Of course, that would break our existing apps and cause headaches.

How about this: create an alternative to "method":"return_json" (return_json2? return_jsend?) where a client can ask for a 'nice' JSON response with status, data, and optional error and message keys. Then we wouldn't break old clients and instead can take the time to update them appropriately. The necessary client update, of course, would simply be to read json.data rather than json.

@bmschmidt
Copy link
Member

The JSend spec looks like the right way: but yes, I've been trying to dodge that implication for the reasons you say.

I like your plan to tweak the API call for back-compatibility.

Rather than introduce a new method, could we begin to phase in the change from this ticket and specify that iff 'format=="json"`in the base query object, it returns in the better format you propose?

@organisciak
Copy link
Member Author

See 72648d2.

I used JSend, and for 'code' I'm using HTTP codes, while 'message' adds more details. Next steps are to investigate database errors to provide more informative responses, and to give errors on unexpected {} responses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants