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

generated stream handler doesn't include context in method argument #173

Closed
stevvooe opened this issue Apr 21, 2015 · 2 comments
Closed

Comments

@stevvooe
Copy link
Contributor

It seems that context is correctly passed to unary methods:

... SayHello(context.Context, *HelloRequest) (*HelloReply, error)

However, for streaming handlers, this is part of the response interface (stream in the example below):

... ListFeatures(rect *pb.Rectangle, stream pb.RouteGuide_ListFeaturesServer) error

This seems slightly inconsistent withe usage guidelines of context.Context. If there is a good reason for this, it would be interesting to understand it. There may be cases where we want to modify the context before sending a stream response:

... ListFeatures(ctx context.Context, rect *pb.Rectangle, stream pb.RouteGuide_ListFeaturesServer) error {
    ctx = WithSpecialContext(ctx, ...)
    return stream.Send(ctx, v)
}

This may be particularly useful if a server has the ability to modify headers and trailers based on the context.

@dsymonds
Copy link
Contributor

It's deliberate. There's no need for an extra argument. A context is request-scoped, but so is the generated stream type. It doesn't make sense to talk about modifying the context for returning stream responses. gRPC isn't a general purpose proto/http2 interface, it's for RPCs specifically. There's a metadata package that is for attaching extra information.

@inconshreveable
Copy link

I understand there's no need for it, but from a usability perspective, this was frustrating for me. It's inconsistent with the handler APIs generated for unary RPCs which is disorienting, but it's also inconsistent with the client API for streaming RPCs which take a context as their first parameter just like unary calls.

My initial thought was "do streaming RPC's not support a context? And then I had to chase down documentation the definition of the generated Stream type and then it's embedded interfaces grpc.ServerStream and grpc.Stream before I found it.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants