JSON RPC over HTTP Transport #451
Conversation
|
In general, is jsonrpc tied to HTTP or can it be done over raw TCP too? If so, it'd be nice to have that option. |
|
In general, no it isn't. Technically HTTP is the transport and JSON RPC is the codec. To abstract this even more RPC is is the codec and JSON is the data format, you could have XML I started down that route, but pulled back since I didn't want to get too deep into the rabbit hole before involving maintainers. |
|
Doesn't the std lib RPC library have a jsonrpc codec? If I understand
correctly, that doesn't use HTTP. Is that an outlier then?
…On Tue, Feb 7, 2017 at 8:14 AM Beard of War ***@***.***> wrote:
In general, no it isn't. Technically HTTP is the transport and JSON RPC is
the codec. To abstract this even more RPC is is the codec and JSON is the
data format, you could have XML [image:
 ] or anything else if
there was an interface defined for it.
I started down that route, but pulled back since I didn't want to get too
deep into the rabbit hole before involving maintainers.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#451 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD5VnaP-I8zM5y83pU7ShakxnG3FkP3ks5raJiDgaJpZM4L5rX->
.
|
|
I've never heard of anyone doing JSON/RPC over anything other than HTTP, I wouldn't put that up as a blocker. |
|
Yes it does, however it does not provide the convenience structs for doing it over HTTP The stdlib does't expose the proper request and response object either, especially for decoding/encoding to JSON with struct tags. |
| JSONRPC string `json:"jsonrpc"` | ||
| Method string `json:"method"` | ||
| Params json.RawMessage `json:"params"` | ||
| ID interface{} `json:"id"` |
There was a problem hiding this comment.
Instead of interface{}, the ID could be a custom type that implements MarshalJSON. Thoughts?
|
|
||
| // Decode the body into an object | ||
| var req Request | ||
| err := json.NewDecoder(r.Body).Decode(&req) |
There was a problem hiding this comment.
It's a bit troubling that there is no way to wrap the r.Body by another io.Reader here. For example someone might want to wrap it with a io.LimitReader to ensure that the server is not flooded with a stream of neverending bytes.
Perhaps a ServerOption can be added here.
|
|
||
| res.Result = resParams | ||
|
|
||
| json.NewEncoder(w).Encode(res) |
There was a problem hiding this comment.
Similar to the Decode, but here my request would be to be able to set options on the encoder: Example to pretty print the response, I'd want to SetIndent
|
@groob Thanks for the feedback here. I appreciate it. I will spend more time cleaning up these parts and update the PR. |
|
Just wanted to make it clear I'm +1 on this package existing, and the general direction looks good to me. I'll withhold further review until you get it into a more final state. JSON/RPC is new to me, I'd love to see a comprehensive example as part of the PR. |
|
I would like to see if we can extend the transport's logic to include RPC batch support which is part of the JSON-RPC 2.0 spec. The spec is very friendly in that it is explicit about not enforcing any guarantees in parallelism or response order sequence so having this feature in should not be terribly difficult. |
|
@basvanbeek Yes, supporting the full spec should be trivial. I wanted to get the basics ironed out first since expanding it for batching and notifications should be simple. |
|
Closing in favor of #576 |
5 participants
This is just a first pass at adding a
jsonrpcpackage underhttpas a transport. I haven't written tests yet, since I wanted to gather feedback as to whether this was in the spirit of gokit before continuing along. Feel free to rip it apart and let me know how it looks.This was a awesome learning experience too!