Skip to content

Add gRPC and gRPC-Web coverage for missing terminal status - #1123

Open
EffortlessSteven wants to merge 3 commits into
connectrpc:mainfrom
EffortlessSteven:codex/grpc-terminal-status
Open

Add gRPC and gRPC-Web coverage for missing terminal status#1123
EffortlessSteven wants to merge 3 commits into
connectrpc:mainfrom
EffortlessSteven:codex/grpc-terminal-status

Conversation

@EffortlessSteven

@EffortlessSteven EffortlessSteven commented Aug 1, 2026

Copy link
Copy Markdown

What this does

The client suites did not cover the full gRPC and gRPC-Web operation matrix for a valid response message followed by EOF without terminal grpc-status.

Change: add unary, client-streaming, server-streaming, and half-duplex bidi cases for both protocols. Unary and client-streaming must fail without returning a payload; server-streaming and half-duplex bidi must deliver the valid payload before the terminal error. The browser adapter is unchanged; grpc-web's unary and server-streaming behavior is recorded as two narrow known failures.

Verification

Check Result
reference client 4 operations × 7 server configs, all passed
grpc-go client 1.82.1 4 operations × 1 config, all passed
grpc-web client 1.5.0 2 advertised operations × 1 config: 2 selected, 2 expected failures, 0 unexpected on Node 22
known-failure scope the 2 patterns match exactly those 2 cases; the divergence reproduces on grpc-web 2.0.2
negative controls completing the response or omitting the expected payload made representative cases fail
repository gates make, make checkgenerate, and git diff --check clean in a Linux-native checkout

Review map

  • internal/app/connectconformance/testsuites/data/grpc_client_unexpected.yaml: the four native gRPC cases, terminating with no HTTP trailers.
  • internal/app/connectconformance/testsuites/data/grpc_web_client_unexpected.yaml: the same operation matrix, omitting the in-body trailers frame.
  • testing/grpcwebclient-known-failing.txt: records the official grpc-web divergence for unary and server-streaming, the only operation kinds the client advertises.

Closes #1115.

Signed-off-by: Steven Zimmerman, CPA <15812269+EffortlessSteven@users.noreply.github.com>

@jhump jhump left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Assuming CI checks out (which I assume it will), the only thing I'd want to tweak is the comment in the known-failing file for grpc-web.

Comment thread testing/grpcwebclient-known-failing.txt Outdated
EffortlessSteven and others added 2 commits August 4, 2026 05:29
Signed-off-by: Steven Zimmerman, CPA <15812269+EffortlessSteven@users.noreply.github.com>
@jhump
jhump enabled auto-merge (squash) August 4, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add gRPC and gRPC-Web coverage for missing terminal status

2 participants