Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javascript and/or web support #6

Open
zeebo opened this issue Apr 27, 2021 · 26 comments
Open

Javascript and/or web support #6

zeebo opened this issue Apr 27, 2021 · 26 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@zeebo
Copy link
Collaborator

zeebo commented Apr 27, 2021

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.

@zeebo zeebo added enhancement New feature or request help wanted Extra attention is needed labels Apr 28, 2021
@alecthomas
Copy link

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
Copy link

mjpitz commented May 2, 2021

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
Copy link

mjpitz commented May 3, 2021

@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
Copy link
Collaborator Author

zeebo commented May 3, 2021

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
Copy link
Member

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

@mjpitz
Copy link

mjpitz commented May 4, 2021

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
Copy link

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
Copy link

mjpitz commented May 17, 2021

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
Copy link

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
Copy link
Collaborator Author

zeebo commented Jul 19, 2021

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
Copy link

Amazing. Is this released?

@zeebo
Copy link
Collaborator Author

zeebo commented Jul 19, 2021

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
Copy link

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
Copy link

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
Copy link
Collaborator Author

zeebo commented Jul 20, 2021

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
Copy link
Collaborator Author

zeebo commented Aug 13, 2021

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

@frederikhors
Copy link

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
Copy link

paralin commented May 27, 2022

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
Copy link

paralin commented May 27, 2022

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
Copy link

paralin commented May 27, 2022

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:

  • Implement the RPC interface with a WebSocket (or similar) read/write callbacks
  • This would include a Header packet which has the request ID (for streaming requests), and the service + method + data when starting a new request.
  • Go package implementing a binding to a DRPC mux with read/write callbacks.
  • Go package implementing a WebSocket demo of this message transport. (similar to drpc.Server, consuming drpc.Mux)

I'll have a look at implementing this!

@mjpitz
Copy link

mjpitz commented May 31, 2022

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
Copy link

paralin commented May 31, 2022

@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
Copy link

mjpitz commented May 31, 2022

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
Copy link

paralin commented May 31, 2022

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

@mjpitz
Copy link

mjpitz commented May 31, 2022

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
Copy link

paralin commented May 31, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants