-
Notifications
You must be signed in to change notification settings - Fork 134
Description
Describe the bug
To support the "simple" client interface these lines were added to NewClientStreamHandler. Note: The same lines of code were added to NewBidiStreamHandler. There are two things that make me think this is probably in the wrong place:
- This effectively wraps the context with streamingHandlerCall info AFTER all interceptors have run, rather than before. Meaning interceptors cannot make use of context values.
- This was added to the current stream handler, not the simplified stream handler. The non-simplified handler probably doesn't need the context to be wrapped as it never was before. In contrast, the simplified stream handler absolutely needs the extra information to be wrapped it wouldn't have the extra request/response wrappers. That said I haven't been involved in the planning or development of this feature perhaps I'm missing something as to why this was done here.
For us, the lines of code added were actually a breaking change. In our middleware we construct a custom context that is passed to the server layer as that type. Our middleware makes use of headers and other values in the context so we wouldn't be able to switch to the simplified interface unless the order of events is correct. Wrapping the context AFTER our middleware ran changed the underlying context type unexpectedly. Yes, this is an atypical pattern but is mostly unrelated to the potential issues above.
To Reproduce
The two concerns are outlined above.
- I'd expect request info to be added to the context before passing through middleware.
- And that this would not touch the existing, non-simple service at all.
Beyond that, there's not much to write out as a repro step.
Environment (please complete the following information):
protoc-gen-connect-go:1.16.2go version:go version go 1.25.0 darwin/amd64- your complete
go.mod:
For this issue the entirety of my go.mod file is irrelevant, but I've included all proto-related dependencies:
module github.com
go 1.25.0
require (
github.com/bufbuild/protovalidate-go v0.9.3
github.com/planetscale/vtprotobuf v0.6.1-0.20250313105119-ba97887b0a25
google.golang.org/grpc v1.75.1
google.golang.org/protobuf v1.36.10
)
require (
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.6-20250307204501-0409229c3780.1
connectrpc.com/connect v1.19.0
connectrpc.com/cors v0.1.0
connectrpc.com/grpchealth v1.4.0
connectrpc.com/grpcreflect v1.3.0
connectrpc.com/otelconnect v0.8.0
)Additional context