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

Detach IsGrpcWebRequest and IsGrpcWebSocketRequest from WrappedGrpcServer #1118

Open
mholt opened this issue Jul 30, 2022 · 2 comments
Open

Comments

@mholt
Copy link

mholt commented Jul 30, 2022

The methods IsGrpcWebRequest and IsGrpcWebSocketRequest have WrappedGrpcServer as a receiver, but they do not use WrappedGrpcServer. They could just be functions: grpcweb.IsGrpcWebRequest() etc.

I am integrating this library into a Caddy plugin and being able to call those methods without having to make a WrappedGrpcServer would be ideal. For now I am just copying+pasting the one-line within them.

Edit: Actually, I think what I really want is for a way to bridge gRPC-web without having to wrap an http.Handler.

Basically, all I need to do is call w.HandleGrpcWebsocketRequest(resp, req) or w.HandleGrpcWebRequest(resp, req) myself within my own http.Handler. The problem is my handler is dynamic and I don't want to make a WrappedGrpcServer for every HTTP request because that's not efficient. Is/can there a way to do the bridging without wrapping a handler?

Another thought: I noticed that the WrappedGrpcServer doesn't actually persist any state between requests. It holds configuration but no extra state. That's excellent! I wonder if we do not need to create a struct that persists for the lifetime of the server then. In other words, could this package be implemented as a config struct with functions like BridgeGRPCWeb() or similar? (As opposed to an entire Server implementation that wraps handlers, etc.)

@johanbrandhorst
Copy link
Contributor

Hi Matt, I just replied in the other issue. Sorry to have taken this long, I hope you haven't spent too much time digging through our internals, as I said in the other issue, I think you're much better off recreating this logic in Caddy directly.

On another note, check out https://connect.build/ if you want to see another implementation of gRPC-Web in Go.

@mholt
Copy link
Author

mholt commented Aug 1, 2022

Thanks! Will do.

I still think the two methods could be detached from the WrappedGrpcServer type for some profit. But not important. :) Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants