client: carry response metadata on every terminal error

[iainmcgin]

Fixes #202

Terminal client errors now carry the response headers whenever they were received, and the trailers whenever they were parsed, regardless of stream shape or protocol.

The inventory in #202 listed four sites where an error dropped metadata its sibling path preserved. Rather than adding set_response_headers at each, the metadata is attached where the terminal record is written, so the rule holds by construction: ServerStream::message() has the only write, and every StreamEnd funnels through it — carried END_STREAM errors, decompression failures, the classify_grpc_end variants and the parse_grpc_error_from_trailers results. The explicit call added by #201 is now redundant and removed. Writing the metadata in rather than applying it on read matters because ServerStream::error() reads the stored record directly. The remaining bare sites on the Connect unary and client-streaming paths are covered too, so "consistently" means what it says.

Item 5 in the issue is untouched; it needs a 'static bound on the body error type and is tracked as #237.

The choke point initially clobbered curated trailers. parse_grpc_error_from_trailers deliberately excludes the gRPC status keys from error metadata, and attaching the raw map on top re-exposed them. Guarding on "already set" would have been unsound — trailers carrying only grpc-status produce a legitimately empty curated map — so the filter is extracted and the choke point recomputes it, which is idempotent on the gRPC path and a no-op on Connect.

The server-side half, and why it is here

Attaching upstream headers to streaming errors would have widened a framing bug that already existed on the unary paths, so it is fixed rather than shipped and filed.

Our server echoed err.response_headers() onto the wire with a builder that appends. Driven against connect-go v1.19.1 and grpc-go v1.81.1, with a per-header bisect, that is not hygiene — it is a hard break. A gateway propagating an upstream failure over Connect on HTTP/1.1 emits a response hyper cannot serialize at all: the connection closes with zero bytes written, connect-go reports unexpected EOF with empty metadata, and a handler's permission_denied reaches the caller as unavailable. Every error code collapses the same way, because all the client ever sees is a transport failure. content-length is the sole culprit. An echoed content-encoding is a second, milder break — connect-go tries to gunzip the plain-JSON error body and falls back to the HTTP status — and an echoed date displaces the one hyper would otherwise set, so the response carries a false generation time.

echo_error_headers applies two rules: a header the response already set wins, and names that would describe something other than this response are dropped — hop-by-hop, body framing, provenance, status-bearing. Legitimate metadata is unaffected: x-upstream-region and both values of a multi-valued x-upstream-tag arrive at every client on every shape, which is the case the snapshot-before-append logic could plausibly have broken.

Dropping grpc-encoding was checked rather than assumed. grpc-go only consults it when a frame's compressed flag is set, and these responses have no data frames, so an echoed one is inert today — confirmed, including under AcceptCompressors("identity"), which makes grpc-go police the advertised encoding. It becomes live the moment a response carries both the echoed encoding and a message frame, and the Connect analogue of the same class already is.

Trailing metadata is still echoed unfiltered. The same class exists one frame later, but fixing it needs a policy rather than a predicate — provenance is not derivable where the filtering happens, the obvious signal is remotely drivable, and the source-selection branch has the same bug one level up. An attempt to fix it here was reverted for those reasons and is now #263.

Verification

Conformance: server 3600/0, server Connect+TLS 2396/0, client Connect 2580/0, client gRPC-Web 2838/0. The gRPC client suite fails 1-4 cases in Timeouts across four runs with a different set each time; base flakes identically, including one clean run.

duplicate_metadata.yaml is the case worth naming — it echoes multi-valued x-custom-header onto error responses across all four RPC kinds. Conformance never asks a server to echo a connection-scoped header, though, and never echoes a name the server also sets, so neither new rule is exercised by the suite; unit tests and the interop harness cover those.

One note on CI: two handler::tests element-budget tests fail on main right now, independently of this change — fixture rot from buffa 0.9.1, fixed by #239.

[iainmcgin]

Filed the conformance gap upstream as connectrpc/conformance#1121: the whole testsuite only ever echoes x-custom-header and x-custom-trailer, which is why this shipped through 3600/0 server conformance.

The upstream issue puts the open question plainly — should a conformant server drop reserved names, forward them verbatim, or reject the RPC — and notes that content-length at least cannot be forwarded, since it yields a response the peer cannot read. The trailer half of the same question is #263 here.