watcher and child chain API specification

[InoMurko]

Our API for all system components is currently speced as:

• API routes
• everything is POST
• http status 200 (no such thing as 400 or 404)
• not all routes need body

The last item is what makes http clients that follow w3c closely incompatible and will refuse to make requests such as <watcher>/status.get because status.get route has no body requirement (w3c POST request have mandatory body).

The question is.. what do we do to make this behavior consistent and interoperable?

-Do we make the change and move all api routes that don't require a body to GET. This forces us to consider whether routes like transaction.all should move it's filter params to URL params?

-Do we revamp the API to REST, or JSON-RPC 2.0 https://www.simple-is-better.org/json-rpc/transport_http.html.

• do what @kevsul said in a comment bellow
• do what @T-Dnzt said in a comment bellow -Or do we just leave this as is.

cc @achiurizo @pdobacz @unnawut

[InoMurko]

We've had a discussion in December about this: I said: As per rfc2616 (http 1.1), a POST request without body is invalid. which means swagger generated code is not able to make a /status.get request, if the generated code uses a strict RFC client

@kevsul said: I believe it’s because the Watcher API is using the JSON RPC protocol (https://www.jsonrpc.org/specification). Every request is a POST. The body shouldn’t be empty though, it should at least contain

{
  "jsonrpc": "2.0",
  "id": 0
}

@pnowosie said: https://github.com/omisego/ewallet/search?q=http-rpc&unscoped_q=http-rpc

@T-Dnzt said: robin and @sirn originally encouraged the eWallet team to go the HTTP-RPC path to avoid tying ourselves to a specific protocol (like HTTP) , allowing the consumption of endpoints via HTTP, websockets or whatever in a similar fashion. Some of the examples we used included Slack API (https://api.slack.com/web) - in retrospect, and now that it's a bit more mainstream, I wished we'd just use GraphQL(with the HTTP adjustments needed ~ https://developer.github.com/v4/) (edited)

[InoMurko]

Should we consider versioning support?

[pnowosie]

Just a comment from my memory when I tested status.get both HTTPie & SwaggerUI were able to call it but swagger's curl example had failed.

Difference is that for the curl one need to specifically add request header content-length: 0 to make it working. The two others added it by default (POST request with empty body -> add content-length: 0)

I'm not here to judge which client follows the standard more strictly :open_hands:. However I don't see much value to change (post -> get)

[unnawut]

I'm keen towards keeping the current convention because:

1. Paths e.g. *.get_balance, *.submit_typed are a lot more customizable and easier to understand. The HTTP verbs are limited and so we'll end up with using the path anyway. So I prefer to avoid thinking in 2 places.

2. No need to figure out different request structures. E.g. whether it needs an empty request body, a structured body and not query strings (UI clients like Postman is pretty bad on making this obvious). This helps a lot when debugging requests where I can rule out malformed request pretty quickly.

3. Likewise for response body, just need to parse once to get success: true/false along with descriptive error message, rather than a 2-step check HTTP response code + parse error message.

Although having said all this, I'm keen to have a single GET endpoint at the root URL for status targeted towards infrastructure toolings and quick human-friendly lookup. Like https://ewallet.staging.omisego.io.

Ah and I'm keen towards JSON-RPC too. We're not that far off anyway.

[InoMurko]

JSON-RPC requirements are (the important ones):

• no URL paths
• all requests have bodies, a body contains the RPC method

[unnawut]

Ah I missed the "no URL paths" part. Quite a big change. 😅

My suggestion would then be:

1. Keep the current convention
2. Allow GET only on the root URL where it returns status data

[pdobacz]

a note on JSON-RPC:

chch and watcher used to do JSON-RPC (in order to be in-line with Ethereum JSON-RPC). We changed to HTTP-RPC in order to be in line with eWallet.

Roughly, moving back to JSON-RPC could just be to move the URL paths to method in JSON-RPC, because that's where they came from :). But not sure how much we drifted away from that JSON-RPC mindset.

.

Pragmatically though, I agree with

My suggestion would then be:

1. Keep the current convention
2. Allow GET only on the root URL where it returns status data

I initially thought about just duplicating status.get with GET verb, but the GET / option sounds more elegant and safe.

[jrhite]

Was just thinking about this topic as I was playing around with DD. They handle RPC errors well, wasn't sure about the always 200 response code thing.

Yeah, RPCs protocols sit on top of HTTP, so they don't need to nor, do they usually conform to RFC2616 and the rules around response codes on errors - particularly client errors.

And yeah, +100 for GraphQL if Elixir has good support for it. 🎈