creachadair / jrpc2

A Go implementation of a JSON-RPC 2.0 server and client.
BSD 3-Clause "New" or "Revised" License
62 stars 10 forks source link

Add a way for the HTTP Bridge to propagate the request context to JSONRPC handlers #99

Closed 2opremio closed 8 months ago

2opremio commented 1 year ago

I am using the HTTP Bridge to serve JSONRPC requests.

In the request Handler I would like to use values from the originating HTTP request context (e.g. the HTTP request ID as injected by https://pkg.go.dev/github.com/go-chi/chi/middleware#RequestID ).

I can see that there is a NewContext Server option but I don't see a way to propagate the HTTP request context there.

creachadair commented 1 year ago

There isn't a great way to do this right now. A jhttp.Bridge shares a single jrpc2.Server across all the HTTP requests delivered by the router, so the server's NewContext hook can't inject per-request values.

In general, there's not much reason to use JSON-RPC in an environment where you already have access to make HTTP or HTTPS requests. JSON-RPC doesn't have any standard notion of request metadata, for example, and inside an HTTP request the version tag, request ID, etc., are redundant when you have headers. A Bridge allows HTTP to act as a transport, but an API that needs the full expressive power of HTTP should probably use that instead of JSON-RPC.

What I'd probably do in this case, instead of using the Bridge, is rev the server API to use plain HTTP handlers.

Then, to permit existing clients to migrate, I'd write a net/http wrapper that just strips off the JSON-RPC wrappers, forwards to the "real" (undecorated) handler, and then repackages the response. So, say, make /v2/<method-name> takes just the JSON "params" and return the JSON response. Then rework the existing /v1/rpc handler to pick open {"jsonrpc":"2.0", "id":<x>, "method":<y>, "params":<z>} and forward it to /v2/<y>, sending Content-Type: application/json body <z>. Once it gets the reply, it can stitch that back into the response payload.

To handle this case in the library, the Channel type would have to be reworked or extended to allow a side-channel, which is certainly possible but I am not convinced it's worth the extra complexity. In this case there are better solutions available. 🙂

2opremio commented 1 year ago

Thanks a lot for the suggestions! I will bring it up with the team!