sourcegraph / jsonrpc2

Package jsonrpc2 provides a client and server implementation of JSON-RPC 2.0 (http://www.jsonrpc.org/specification)
MIT License
190 stars 62 forks source link

[Feature Request] Add a way to specify more non-standard-compliant fields to Request #49

Closed lhchavez closed 3 years ago

lhchavez commented 3 years ago

Hi!

Thanks for open-sourcing this library, which seems to be the only one that plays nicely with WebSockets :D

This may be a somewhat niche request, but would it be reasonable to ask for a way to have the Request have more non-standard-compliant fields at the top (sorry, I know this is awful and opens a big can of worms u__u)? The rationale is that there's this one JSON-RPC2-based protocol that adds a couple more fields outside of params (namely, sessionId and pauseId): https://replay.io/protocol

I can make the change (with tests) if this request is deemed to be within the scope of this library. I'm planning on adding a ExtraFields []struct{name string, value *json.RawMessage} `json:"-"` and a func (r *Request) SetExtraField(name string, value interface{}) error to Request, and that way a new ClientOpt can add this to the Request.

keegancsmith commented 3 years ago

Hi @lhchavez. I like this idea. In fact we used to do this as well internally in our usage of this library to add our own extra fields (eg tracing). We didn't use an approach like this, I can't remember how we did it and don't feel like doing code archealogy right now :). But cc @sqs to see if he can remember. However, your approach looks nice so I am happy to proceed with this API addition.

I took a look at the referenced commit and it LGTM. I wonder if you can avoid the extra json.Unmarshal you add to find the extra fields? IE do a similiar trick you do for marshalling and switch from a struct to a map.

Happy to discuss further in a PR.