Opened 3 years ago

Closed 3 years ago

#21036 closed defect (not a bug)

Onionoo doesn't return JSON for error responses

Reported by: lukechilds Owned by: metrics-team
Priority: Low Milestone:
Component: Metrics/Onionoo Version:
Severity: Minor Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

From the Onionoo docs:

Onionoo clients send HTTP GET requests to the Onionoo server which responds with JSON-formatted replies.

Clients should be able to handle all valid JSON responses

https://onionoo.torproject.org/protocol.html#general

However Onionoo only returns JSON on a successful response, errors are returned in HMTL. For example:

Returns JSON:
https://onionoo.torproject.org/summary?limit=1

Returns HTML:
https://onionoo.torproject.org/summary?limit=foo

The HTTP lib I'm using in my Onionoo client library is configured for JSON so it automatically parses the response for me. This means when I get an error (400 Bad Request in the above example) I don't get a chance to throw a useful bad request request error as the HTTP lib has already thrown a parse error.

I've implemented a fix by configuring my HTTP lib for text responses and then manually parsing the JSON myself, only for successful responses. This obviously isn't a major issue but I think it would be better (and match the spec) if Onionoo returned errors formatted as JSON. If that's going to be an issue it should mention that errors are returned as HTML in the spec.

Child Tickets

Change History (3)

comment:1 Changed 3 years ago by karsten

Interesting. I wonder whether you (or in this case your HTTP library) should even attempt to parse the body of a 400 response. It seems potentially problematic with many other web services.

I'm not yet convinced though that we should return JSON responses for error cases. We'd have to extend the JSON format just for this, which seems like overkill. In theory, returning an HTTP error code should be sufficient for clients.

So, I agree that it might be confusing to return an HTML body in these error cases and a JSON response in the success case. I guess that's a matter of configuring Jetty to not provide HTML output together with the HTTP 400 response and instead provide an empty body. Would that be better? And would browsers (and browser users) be okay with that?

comment:2 Changed 3 years ago by iwakeh

The Onionoo protocol defines 400 as reply to bad requests. The returned data simply shouldn't be parsed.
A general server error would also return html and clients should be able to handle that too.

Currently, Onionoo doesn't have a more elaborate debugging/error messaging protocol part defined. Requiring JSON as reply to malformed requests is in the end requiring a 'debugging extension' to the protocol, i.e., error code etc.

comment:3 Changed 3 years ago by lukechilds

Resolution: not a bug
Status: newclosed

Yeah fair enough, I've just a had a look around and found a few other APIs that also return HTML for errors. Just using the response codes seems perfectly reasonable.

I still think JSON errors would be preferable but I suppose this is more of a design decision than a bug.

Note: See TracTickets for help on using tickets.