-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to connectrpc.com/connect v1.14.0 and google.golang.org/protobuf v1.32.0 #111
Changes from 5 commits
1de4fca
c3dd014
e6323e0
6e6e92b
7c03a8f
3885bf3
4779159
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emcfarlane, this makes me a little nervous. This smells like we inadvertently introduced a backwards-incompatible change in connect-go. Maybe you can elaborate a little on "the new Send operation", and why this change was really needed, and why that's not a bug in connect-go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep sure. The bytes.Buffer
WriteTo
won'tWrite
to theio.Writer
if the payload size is zero: https://cs.opensource.google/go/go/+/refs/tags/go1.21.5:src/bytes/buffer.go;l=260Vanguard-go uses the
Write
to trigger the conversion of an empty protomessage (valid zero length payload) to a json message ({}
). I do think this is an issue with howvanguard-go
triggers conversions. It shouldn't be dependant on the write mechanism. This was the minimal change I could see to fix the issue but I would presume we wish to revisit later.