codegen: carry mixed gRPC streaming requests in typed envelopes#3918
Merged
codegen: carry mixed gRPC streaming requests in typed envelopes#3918
Conversation
Stop rewriting ordinary payload into metadata for methods that also stream payload items. Generate a typed initial-payload/stream-item envelope and update the gRPC tests and docs around the new transport contract.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #3869.
Today, gRPC methods that define both
Payload(...)andStreamingPayload(...)do not preserve the ordinary method payload as a typed request message. During endpoint finalization, Goa rewrites that payload into request metadata instead.That transport contract is problematic for a few reasons:
OneOfrepro in OneOf Type in Streaming Payload Generates Invalid Code #3869The concrete failure mode in the repro is that a union payload field paired with
StreamingPayload(...)generates invalid gRPC code because the payload is treated as metadata even though the metadata path only really supports primitive-like string conversions.Design Goal
The goal here is not to patch the metadata path to support more cases. That would preserve the wrong abstraction.
Instead, this change makes the gRPC contract conceptually match the service contract:
GRPC.Metadata(...)declarations and security/auth fieldsFor gRPC methods that define both
Payload(...)andStreamingPayload(...), the transport now uses a typed streamed request envelope with two variants:initial_payloadstream_itemThat keeps all non-metadata request data in the protobuf message channel and aligns gRPC more closely with how Goa already models this shape in other streaming transports.
What Changed
Endpoint finalization
In
expr/grpc_endpoint.go:Payload -> Metadatapromotion for methods withStreamingPayload(...)This is the root contract change.
Protobuf/codegen model
In
grpc/codegen/service_data.goand the gRPC templates:The generated protobuf stream request now carries:
initial_payloadframe to open the logical requeststream_itemframes for the actual stream contentsClient behavior
Updated the client open/send path so that:
Send(...)calls emit onlystream_itemenvelope framesServer behavior
Updated the server path so that:
EndpointInput{Payload, Stream}Recv()So the service contract stays stable even though the transport contract becomes correct.
Why This Design
This is a fairly deep change, but it simplifies the model rather than adding more special cases.
Benefits:
This also avoids growing more metadata-specific encode/decode logic for cases that were never really metadata in the first place.
Behavioral Impact
This intentionally changes the gRPC wire contract for methods that define both
Payload(...)andStreamingPayload(...).What stays the same:
GRPC.Metadata(...)fields still use metadataPayloadplus typed stream item send/recvWhat changes:
That is a deliberate contract correction rather than a backward-compatible patch.
Tests And Validation
Added/updated coverage for:
expr/grpc_endpoint_test.goexpr/testdata/endpoint_dsls.gogrpc/codegen/streaming_test.gogrpc/codegen/testdata/dsls.gogrpc/codegen/testdata/goldendsl/payload.go,dsl/grpc.go, andgrpc/docs/FAQ.mdTest Plan
go test ./expr ./grpc/codegen ./dslmake