Feedback

Signed-off-by: Steve Ayers <[email protected]>

Author: Steve Ayers

client.go

@@ -79,7 +79,7 @@ func NewClient[Req, Res any](httpClient HTTPClient, url string, options ...Clien
 		conn := client.protocolClient.NewConn(ctx, unarySpec, request.Header())
 		conn.onRequestSend(func(r *http.Request) {
 			request.setRequestMethod(r.Method)
-			callInfo, ok := getClientCallInfoFromContext(ctx)
+			callInfo, ok := clientCallInfoFromContext(ctx)
 			if ok {
 				callInfo.method = r.Method
 			}
@@ -116,7 +116,7 @@ func NewClient[Req, Res any](httpClient HTTPClient, url string, options ...Clien
 		protocolClient.WriteRequestHeader(StreamTypeUnary, request.Header())
 
 		// Also set them in the context if there's a call info present
-		callInfo, callInfoOk := getClientCallInfoFromContext(ctx)
+		callInfo, callInfoOk := clientCallInfoFromContext(ctx)
 		if callInfoOk {
 			callInfo.peer = request.Peer()
 			callInfo.spec = request.Spec()
@@ -158,19 +158,6 @@ func (c *Client[Req, Res]) CallUnary(ctx context.Context, request *Request[Req])
 	return c.callUnary(ctx, request)
 }
 
-// CallUnarySimple calls a request-response procedure using the function signature
-// associated with the "simple" generation option.
-//
-// This option eliminates the [Request] and [Response] wrappers, and instead uses the
-// context.Context to propagate information such as headers.
-func (c *Client[Req, Res]) CallUnarySimple(ctx context.Context, request *Req) (*Res, error) {
-	response, err := c.CallUnary(ctx, NewRequest(request))
-	if response != nil {
-		return response.Msg, err
-	}
-	return nil, err
-}
-
 // CallClientStream calls a client streaming procedure.
 func (c *Client[Req, Res]) CallClientStream(ctx context.Context) *ClientStreamForClient[Req, Res] {
 	if c.err != nil {
@@ -187,7 +174,7 @@ func (c *Client[Req, Res]) CallServerStream(ctx context.Context, request *Reques
 	if c.err != nil {
 		return nil, c.err
 	}
-	callInfo, callInfoOk := getClientCallInfoFromContext(ctx)
+	callInfo, callInfoOk := clientCallInfoFromContext(ctx)
 	// Set values in the context if there's a call info present
 	if callInfoOk {
 		// Copy the call info into a sentinel value. This is so we can compare
@@ -231,15 +218,6 @@ func (c *Client[Req, Res]) CallServerStream(ctx context.Context, request *Reques
 	}, nil
 }
 
-// CallServerStreamSimple calls a server streaming procedure using the function signature
-// associated with the "simple" generation option.
-//
-// This option eliminates the [Request] wrapper, and instead uses the context.Context to
-// propagate information such as headers.
-func (c *Client[Req, Res]) CallServerStreamSimple(ctx context.Context, requestMsg *Req) (*ServerStreamForClient[Res], error) {
-	return c.CallServerStream(ctx, NewRequest(requestMsg))
-}
-
 // CallBidiStream calls a bidirectional streaming procedure.
 func (c *Client[Req, Res]) CallBidiStream(ctx context.Context) *BidiStreamForClient[Req, Res] {
 	if c.err != nil {

cmd/protoc-gen-connect-go/internal/testdata/simple/gen/genconnect/simple.connect.go

@@ -384,15 +384,19 @@ func generateClientMethod(g *protogen.GeneratedFile, method *protogen.Method, na
 		g.P("return c.", unexport(method.GoName), ".CallClientStream(ctx)")
 	case !isStreamingClient && isStreamingServer:
 		if simple {
-			g.P("return c.", unexport(method.GoName), ".CallServerStreamSimple(ctx, req)")
+			g.P("return c.", unexport(method.GoName), ".CallServerStream(ctx, ", connectPackage.Ident("NewRequest"), "(req))")
 		} else {
 			g.P("return c.", unexport(method.GoName), ".CallServerStream(ctx, req)")
 		}
 	case isStreamingClient && isStreamingServer:
 		g.P("return c.", unexport(method.GoName), ".CallBidiStream(ctx)")
 	default:
 		if simple {
-			g.P("return c.", unexport(method.GoName), ".CallUnarySimple(ctx, req)")
+			g.P("response, err := c.", unexport(method.GoName), ".CallUnary(ctx, ", connectPackage.Ident("NewRequest"), "(req))")
+			g.P("if response != nil {")
+			g.P("return response.Msg, err")
+			g.P("}")
+			g.P("return nil, err")
 		} else {
 			g.P("return c.", unexport(method.GoName), ".CallUnary(ctx, req)")
 		}

cmd/protoc-gen-connect-go/main.go

@@ -368,9 +368,8 @@ type hasHTTPMethod interface {
 	getHTTPMethod() string
 }
 
-// errStreamingClientConn is a sentinel error implementation of StreamingClientConn
+// errStreamingClientConn is a sentinel error implementation of StreamingClientConn.
 type errStreamingClientConn struct {
-	StreamingClientConn
 	err error
 }
 

connect.go

@@ -232,13 +232,13 @@ func (w *responseWrapper[Res]) ResponseTrailer() http.Header {
 	return w.response.Trailer()
 }
 
-// Gets a client (i.e. outgoing) call info from context.
-func getClientCallInfoFromContext(ctx context.Context) (*clientCallInfo, bool) {
+// clientCallInfoFromContext gets the call info from a client/outgoing context.
+func clientCallInfoFromContext(ctx context.Context) (*clientCallInfo, bool) {
 	info, ok := ctx.Value(clientCallInfoContextKey{}).(*clientCallInfo)
 	return info, ok
 }
 
-// newHandlerContext creates a new handler (i.e. incoming) context.
+// newHandlerContext creates a new handler/incoming context.
 func newHandlerContext(ctx context.Context, info CallInfo) context.Context {
 	return context.WithValue(ctx, handlerCallInfoContextKey{}, info)
 }

context.go

@@ -114,23 +114,25 @@ func (c *chain) WrapStreamingHandler(next StreamingHandlerFunc) StreamingHandler
 
 func unaryThunk(next UnaryFunc) UnaryFunc {
 	return func(ctx context.Context, req AnyRequest) (AnyResponse, error) {
-		if !checkSentinel(ctx) {
-			return nil, errNewClientContextProhibited
+		if err := checkSentinel(ctx); err != nil {
+			return nil, err
 		}
 		return next(ctx, req)
 	}
 }
 
 func streamingClientThunk(next StreamingClientFunc) StreamingClientFunc {
 	return func(ctx context.Context, spec Spec) StreamingClientConn {
-		if !checkSentinel(ctx) {
-			return &errStreamingClientConn{err: errNewClientContextProhibited}
+		if err := checkSentinel(ctx); err != nil {
+			return &errStreamingClientConn{err: err}
 		}
 		return next(ctx, spec)
 	}
 }
 
-func checkSentinel(ctx context.Context) bool {
-	// Only verify if there's a sentinel call info to compare it to
-	return ctx.Value(clientCallInfoContextKey{}) == ctx.Value(sentinelContextKey{})
+func checkSentinel(ctx context.Context) error {
+	if ctx.Value(clientCallInfoContextKey{}) != ctx.Value(sentinelContextKey{}) {
+		return errNewClientContextProhibited
+	}
+	return nil
 }