Clarify documentation for half-duplex behaviour

AThilenius commented 5 months ago

Describe the bug

Hi. I'm the author of axum-connect, which supports server-streaming RPC handlers. I am however running in circles trying to figure out what the official spec is for the request part of a server-streaming (IE. unary request, streaming response) RPC. I implemented axum-connect according to the official protocol spec, which defines an RPC call as

Request → Unary-Request / Unary-Get-Request / Streaming-Request ... Response → Unary-Response / Streaming-Response

This implies, though does not actually state, that server-streaming RPC requests are unary (do not have envelops). However, unless I'm misusing it somehow, connect-es is enveloping the request JSON, despite defining the RPC type itself as a unary.

To Reproduce

Here is a minimal web client example of calling Eliza Introduce. The only interesting snippet is

import { createConnectTransport } from "@bufbuild/connect-web";
import { ElizaService } from "@buf/connectrpc_eliza.bufbuild_connect-es/connectrpc/eliza/v1/eliza_connect";
import { createPromiseClient } from "@bufbuild/connect";

const transport = createConnectTransport({
    baseUrl: "https://connect.build/demo",
});
const client = createPromiseClient(ElizaService, transport);

async function test() {
    for await (const response of client.introduce({ name: "Bob Ross", })) {
        console.log(response);
    }
}

void test();

It runs a CORS error if you point it to the demo URL, but you can see the request contains than envelope message. I would only expect this for a streaming request. Otherwise I would expect the request to be normal JSON.

srikrsna-buf commented 5 months ago

Hey! Thank you for creating axum-connect, it is great to see a rust implementation!

The Streaming RPCs section outlines all the streaming RPC types. Server steaming, Client streaming, and Bidi Streaming all use enveloped messages.

Transferred the issue here to track the verbiage change in the outline section if needed.

AThilenius commented 5 months ago

Most likely I've made a bad assumption, there is unfortunately still a lot of ambiguity in the spec.

First Assumption

From the spec:

Streaming RPCs may be half- or full-duplex. In server streaming RPCs, the client sends a single message and the server responds with a stream of messages. In client streaming RPCs, the client sends a stream of messages and the server responds with a single message.

My interpretation of that is that a half-duplex streams will be comprised of a unary part (the "sends a single message") and a streaming part (the "server response with a stream of messages").

My extrapolation of that reading for server-streaming RPCs is that:

Calling-semantics would be unary (not enveloped) but as noted:
- A connect+* content type must be present that "prevents HTTP clients unaware of Connect's semantics from interpreting a streaming error response, which uses an HTTP Status of 200 OK, as successful"
Response semantics are streaming (enveloped with flags and content length, 200-status, et. al).

Second Assumption

My second assumption is that I'm correctly understanding connect-es is indeed enveloping the unary part of half-duplex requests. Part of me wonders if I'm just "holding it wrong" still 🤔

Spec Ambiguity

The spec doesn't actually say either way, including in the ABNF, it depends on how you read it. If the interpretation is instead "all streaming RPCs, client-streaming, server-streaming and bidi-streaming use streaming wire semantics both directions" and for example server-streaming requests just happen to expect exactly one payloads in the stream before the HTTP request terminator, then that is not defined anywhere. Nor the semantics for degenerate cases like request payload timeouts (are they considered unary, or streaming), content-size disagreement, more than one payload, stream termination without HTTP request completion, et. al.

emcfarlane commented 5 months ago

@AThilenius thanks for raising the issue, this also caught me by surprise when I first read through the spec and will look at making this more obvious. From the spec:

Streaming RPCs may be half- or full-duplex.

The terms "server streaming" and "client streaming" are half streaming protocols but the streaming RPC behaviour is defined for both the request and response. A half-streaming protocol is streaming. So there is no unary part. This looks like:

unary: single request and response without message enveloping
server streaming: a single enveloped message request with a stream of zero or more response messages
client streaming: a stream of requests met with single response in a streaming envelope
full duplex: stream of requests and responses intermix (requires http2)

AThilenius commented 5 months ago

@emcfarlane Appreciate the response! It's really good to have a definitive answer, I'll get axum-connect patched soon. Apparently no one is using streaming in axum-connect yet (myself included), so it has gone unnoticed lol.

It's a really surprising answer for me though. I feel like it leaves so much undefined behavior for the single-message parts of the streams. Rust is suuuuper strict, so I'll have to pick what feels like sane defaults myself and define all the undefined behavior in the spec. C'est la vie.

emcfarlane commented 5 months ago

You might find the new conformance tooling useful. It's just been developed to help implementers assert correctness: https://github.com/connectrpc/conformance

AThilenius commented 5 months ago

You might find the new conformance tooling useful. It's just been developed to help implementers assert correctness: https://github.com/connectrpc/conformance

That is perfect, I've been wanting something like that since first creating axum-connect, didn't exist at the time though. Appreciate the link!

AThilenius commented 5 months ago

As for this issue I'm good with either closing it (my question has been answered) or turning it into a 'improve the docs' tracking issue. Up to you guys.

connectrpc / connectrpc.com