storj / drpc

drpc is a lightweight, drop-in replacement for gRPC
MIT License
1.5k stars 50 forks source link

Javascript and/or web support #6

Open zeebo opened 3 years ago

zeebo commented 3 years ago

A commonly requested language is javascript. I haven't used gRPC with javascript before, so I don't know much about use cases, so here's some questions:

  1. Is server support desired?
  2. Is bi-directional streaming support desired?
  3. Does gRPC currently do bi-directional streaming?
  4. Is it targeting being run in a browser or is that "web" which is distinct from "javascript" and/or "node"?
  5. There's an example that shows how to serve both http and raw sockets on the same port, but only handles unitary http requests with json. Would that be sufficient server side support?

This issue is meant to collect information about use cases people have and potential answers to the above questions.

alecthomas commented 3 years ago

My 2c is that unless your goal is to expose the DRPC service using Google's HTTP annotations, I'd go for full feature compatibility and use WebSockets (or whatever the latest hotness is).

mjpitz commented 3 years ago

I managed to get a little done for the wire format for NodeJS here: https://github.com/mjpitz/drpc-node

I'm mostly following the Golang implementation, but I've also been popping over to the wiki as I write it to see how much aligns.

mjpitz commented 3 years ago

@zeebo per your comment on #7, is it ok that the node library is written in typescript? I don't mind swapping, but it's just my preference since it makes reasoning about types a bit easier.

zeebo commented 3 years ago

Yeah, that's totally fine. I prefer typescript as well, but most importantly since you're writing it you get to choose those sorts of things.

egonelbre commented 3 years ago

I think it was possible to use TypeScript code from JavaScript, so I don't think it makes a big difference.

mjpitz commented 3 years ago

Alright. @zeebo I wrapped up the last of the drpcwire package in Node. There's one bug right now where I reset the ID after successfully processing a packet, but that's an easy fix. I've updated the wiki on the wire protocol to describe the frame format a bit more and even included a section on assembling packets. Not sure what the best way to collab on wiki pages in GitHub are or if you have any preference. Anyway, I'll probably start in on the drpcstream package next.

timostamm commented 3 years ago

Hey there, I wrote a protoc-plugin and protobuf runtime that supports gRPCweb and Twirp in the browser and node.

Generated clients delegate work to a facade, so it is relatively easy to implement protocols. For example, adapting gRPC is about 300 loc.

This might save you the trouble of writing a protoc plugin to generate clients. HTH

mjpitz commented 3 years ago

Made some good progress on this during a rainy Sunday. I've got what looks like a possibly workable socket layer? When I've got some time, I'll throw together a directory with an interop test that spins up a go server and tries to use node to communicate with it via the wire protocol (and vice-versa). It would be great to get some eyes on the socket and stream layer just to make sure it looks right from an authors' perspective.

Here's a PR I have up with the proposed socket package: https://github.com/mjpitz/drpc-node/pull/4/files

There are two files in there (stream.ts and a socket.ts) with the bulk of the packet handling and state machine logic that I'd love an extra set of eyes on just to make sure I'm not missing anything.

frederikhors commented 3 years ago

Do you think we can use https://github.com/improbable-eng/grpc-web/tree/master/go/grpcweb with drpc?

If yes with the work @timostamm did (https://github.com/timostamm/protobuf-ts) we are ready for web clients!

zeebo commented 3 years ago

So I tried to make https://pkg.go.dev/storj.io/drpc/drpchttp somewhat follow what I thought Twirp does, but I haven't done any testing to make sure. That may already work if you don't need client streaming. I wonder how hard it would be to extend that package to also serve grpc-web style.

edit: I have a change out at https://review.dev.storj.io/c/storj/drpc/+/5264 that should ensure that drpchttp is compatible with twirp clients going forward.

frederikhors commented 3 years ago

Amazing. Is this released?

zeebo commented 3 years ago

~I pushed it up as the exp/twirp-compat branch so you can experiment with it until it's been reviewed and merged onto main. Using go get storj.io/drpc@exp/twirp-compat should be all that's needed to use it with Go modules.~ It is now merged onto main.

I've also made progress on having drpchttp also serve grpc-web compatible requests/responses, but it's a little bit gross. I'm going to work on finding some ways to clean it up first.

frederikhors commented 3 years ago

I've also made progress on having drpchttp also serve grpc-web compatible requests/responses, but it's a little bit gross. I'm going to work on finding some ways to clean it up first.

Did you get a cue from https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md?

frederikhors commented 3 years ago

I would like to try directly with grpc-web because we already have it in production using classic grpc. I can be your immediate and very fast and detailed beta tester.

zeebo commented 3 years ago

Did you get a cue from https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md?

Yes, partly that, and partly reading various implementations.

The grossness I spoke of is due to the implementation strategy I've so far "chosen" where the drpchttp package has to know about all of the different protocols, rather than being extensible. I'm going to spend some time to try to work out the correct interfaces to make sure all of these things can be done externally if necessary.

zeebo commented 3 years ago

I forgot to mention, grpc-web support should work now that https://github.com/storj/drpc/commit/ebd70c186e6c2c5e18ace6d1db4f5696399b3102 is merged.

frederikhors commented 3 years ago

I forgot to mention, grpc-web support should work now that ebd70c1 is merged.

I will try it in a few days. It would be amazing to have a simple example.

paralin commented 2 years ago

Is there any implementation of calling RPC functions via a message channel (i.e. WebSocket) with a code-genned TypeScript client for the RPCs defined in protobuf?

That seems like it would be quite useful: construct the client with a Conn, same as in Go, which can use type-safe RPC calls over the Conn to the Go server.

paralin commented 2 years ago

I suppose the easiest way to implement what I described above would be;

This is fairly straightforward and would speak the grpc-web HTTP protocol instead of the DRPC protocol.

But, I think it would be interesting to introduce some kind of protobuf-based binary protocol to do this. instead of wasting time formatting HTTP requests, all RPC information (arguments + the function name) could be sent as an encoded Protobuf object.

paralin commented 2 years ago

Towards implementing the protobuf-only RPC exchange over a message channel:

https://github.com/stephenh/ts-proto

For a service:

syntax = "proto3";
package web.demo;

service DemoService {
  rpc DemoEcho(DemoEchoMsg) returns (DemoEchoMsg) {}
}

message DemoEchoMsg {
  string msg = 1;
}

Generates a RPC interface like:

export interface DemoService {
  DemoEcho(request: DemoEchoMsg): Promise<DemoEchoMsg>
}

export class DemoServiceClientImpl implements DemoService {
  private readonly rpc: Rpc
  constructor(rpc: Rpc) {
    this.rpc = rpc
    this.DemoEcho = this.DemoEcho.bind(this)
  }
  DemoEcho(request: DemoEchoMsg): Promise<DemoEchoMsg> {
    const data = DemoEchoMsg.encode(request).finish()
    const promise = this.rpc.request('web.demo.DemoService', 'DemoEcho', data)
    return promise.then((data) => DemoEchoMsg.decode(new _m0.Reader(data)))
  }
}

Where the RPC interface that would need to be implemented is:

interface Rpc {
  request(
    service: string,
    method: string,
    data: Uint8Array
  ): Promise<Uint8Array>
  clientStreamingRequest(
    service: string,
    method: string,
    data: Observable<Uint8Array>
  ): Promise<Uint8Array>
  serverStreamingRequest(
    service: string,
    method: string,
    data: Uint8Array
  ): Observable<Uint8Array>
  bidirectionalStreamingRequest(
    service: string,
    method: string,
    data: Observable<Uint8Array>
  ): Observable<Uint8Array>
}

So, at least the client end is already implemented in ts-proto.

I suppose the next step is to add a new repository which has:

I'll have a look at implementing this!

mjpitz commented 2 years ago

Hey @paralin! I've got the start to an implementation here: https://github.com/mjpitz/drpc-node

The drpcsocket layer tries to work with a generic reader / writer implementation so it should be able to work with a websocket connection pretty easily: https://github.com/mjpitz/drpc-node/blob/main/drpcsocket/socket.ts

The project has a quick integration test to show that it works with drpc-go: https://github.com/mjpitz/drpc-node/tree/main/integration_tests

There's still quite a bit to do. I had hit the point where I needed to start to bridge the code generation layer with the protocol layer and started to look into protobuf generation. TBH, I'm not a huge fan of the implementations that are out there. So I'm tempted to go through and write a custom, pure NodeJS implementation of protobuf that works similar to many of the other languages (unlike the current ones).

Edit: ts-proto looks really promising

paralin commented 2 years ago

@mjpitz I've come up with a very simple way to implement it leveraging ts-proto:

https://github.com/paralin/ts-drpc

I'll have a look at your implementation too, thanks for sharing. Would be interested to hear your thoughts on my approach.

mjpitz commented 2 years ago

Ignore me... I just realized I was writing my library in typescript and was planning on compiling over to JavaScript for publishing....

From your README, the interfaces look similar to the ones that I was going for on the other end of things.

paralin commented 2 years ago

@mjpitz I think the main difference is the focus on web-browser support as opposed to Node.JS.

mjpitz commented 2 years ago

Right... but you should be able to do that with my drpc implementation as well... as long as the websocket implements the reader and writer interfaces. I do like that proto library though... it looks really promising.

My demos currently show a server-side client, but I could definitely see swapping that out for a WebSocket.

paralin commented 2 years ago

@mjpitz It's really impressive that you've implemented the drpcwire format there. I think the two have slightly different goals; you're implementing DRPC as a fully wire-compatible format, while I'm just making a separate protocol specifically for a Js frontend to route RPC requests to a Go backend via a WebSocket, not necessarily to interface with vanilla DRPC protocol servers.

I think it would make sense to add an implementation of the ts-proto Rpc interface (posted above) to your drpc-node repo which uses the DRPC protocol.