search

awakesecurity / gRPC-haskell

Haskell gRPC support
https://hackage.haskell.org/package/grpc-haskell
Apache License 2.0
234 stars 73 forks source link

Document why `ServerReaderResponse` takes a `Maybe response` #13

Open crclark opened 7 years ago

crclark commented 7 years ago

I just came across this and can't remember why it's optional to return a response message to a client streaming RPC.

The gRPC docs don't mention that the server's response message is optional. It seems to me that either both unary and client streaming responses should be optional, or neither should be optional.

Either way, it should be documented.

ixmatus commented 7 years ago

HTTP has a notion of a successful but empty bodied response: the 204 No Content. It wouldn't surprise me if gRPC had a similar response code since it's based on HTTP2. Nothing for an empty bodied but successful response makes a lot of sense to me, though it's a bit funky for the other responses that might expect a non-empty response body.

intractable commented 7 years ago

Gah, I'm pretty sure there was a reason for this -- I thought that perhaps because of a usage pattern we noticed in some of the C++ client-streaming examples. However, when I went and looked I couldn't reconstruct it. I'm not sure it makes sense either, given that the rpc definition for client-streaming endpoints always specifies a response type. And, as @crclark said, I'm not aware of anything that claims a response message is optional.

I think we should try dropping the Maybe and check interop against the C++ versions as a sanity check. I'll assign this to myself for the time being; will try to look at it soon.

crclark commented 7 years ago

Looking at the types again, I agree with @ixmatus that the type seems to be accounting for status codes that don't include a response message, and all of our high level types should have Maybe responses. Specifically, we have status codes like UNAUTHENTICATED, but the type of the high level unary RPCs guarantees a response message, which seems wrong prima facie. I'm thinking the high level types need to be redesigned around the assumption that we can receive status codes without a response.

This is kind of related to https://github.com/awakenetworks/gRPC-haskell/issues/3 (I'm going to add some detail to that issue, too).