-
Notifications
You must be signed in to change notification settings - Fork 12
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
Comments
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? |
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" |
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: How about this: create an alternative to |
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 |
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. |
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
facetName
doesn't existdatabaseName
does not existServer Error (i.e. please contact
admin
)6. Cannot access host
7. Memory tables need to be rebuilt
8. Unknown server error
The text was updated successfully, but these errors were encountered: