connect-go icon indicating copy to clipboard operation
connect-go copied to clipboard

Add limited support for bidi streams over HTTP/1

Open emcfarlane opened this issue 1 year ago • 1 comments

This adds limited support for bidirectional streams over HTTP/1.x transports. It removes the explicit error response which disabled this behavior based on the protocol stream type. Bidi streams may now be used in half-duplex mode. This does not enable full-duplex streaming. All requests must be sent before responses are received.

This removes the workarounds in the conformance test suite to fake a HTTP/2 request/response in order to allow for this behavior. In doing so a deadlock on writing to a closed requests was fixed. See https://github.com/connectrpc/conformance/pull/942.

Edit: Will look at improving the error messages on trying to do a full-duplex stream over HTTP/1.x. It should fail noting this isn't possible.

emcfarlane avatar Nov 08 '24 20:11 emcfarlane

I'd originally forbidden this because I thought it was too difficult to use correctly.

Clients are often created with an HTTPS URL (e.g., https://api.connectrpc.com). From that URL, it's not clear whether the server supports HTTP/2. In itself, that's okay. But per my recollection, attempting full-duplex streaming over HTTP/1.1 just hangs. The error message is also unhelpful - my memory is a little vague, but I think the client just gets a timeout, or a cancellation, or something equally unhelpful. To me, this felt like an easy trap to stumble into, and a difficult error to diagnose.

I'm open to allowing half-duplex streaming over HTTP/1.1, but I'd like to make sure that attempting full-duplex communication results in a clear error message.

akshayjshah avatar Jan 08 '25 19:01 akshayjshah

Closing, this needs testcases for error cases, but will pick up at a later time.

emcfarlane avatar Jun 24 '25 18:06 emcfarlane