define Request.GetBody to avoid this error

[pqn]

Describe the bug

In some of our production logs we've noticed the following client error:

unavailable: http2: Transport: cannot retry err [http2: Transport received Server's graceful shutdown GOAWAY] after Request.Body was written; define Request.GetBody to avoid this error

To Reproduce

We've been unable to reproduce this locally.

Environment (please complete the following information):

• connect-go version or commit: v1.6.0
• go version: go version go1.20.5 linux/amd64
• your complete go.mod: I can provide dependencies as needed

Additional context

• We are using the GRPC protocol, and the server is a Python gRPC server.
• It seems related to this issue: Post es http err:http2: Transport: cannot retry err [http2: Transport received Server's graceful shutdown GOAWAY] after Request.Body was written; define Request.GetBody to avoid this error olivere/elastic#1443
• One way this has been fixed in a public repo is here: Set Request.GetBody stripe/stripe-go#711

[emcfarlane]

Hey @pqn! Thanks for the detailed breakdown. For unary requests we should be able to set GetBody on requests as we have the body in a bytes.Buffer. However we currently write to the body with a pipe: https://github.com/bufbuild/connect-go/blob/main/duplex_http_call.go#L80
I'll try replicate this with testing and see if we can create a fix for unary requests. In the meantime you might be able to check for this error and retry on the client side. How often do you see this happening?

[pqn]

In one log I'm seeing it maybe 4-5 times in an hour.

[espadolini]

We see this error 10-40 times per minute in our prod infra out of roughly 2400 requests, so 0.4% to 1.6% of all requests are being retried, which isn't terrible but also not insignificant.

[pqn]

I saw #543 was closed. Just wondering, is this being worked on or in the backlog right now? Thanks!

[emcfarlane]

Hey @pqn, we are still aiming to fix this issue soon. The feedback on the PR was to focus on the unary changes first as this will have the largest impact. @mattrobenolt has some good suggestion on splitting out the unary flow to get some nice performance improvements too. Will be a much neater solution!

[pqn]

Sounds good! Thanks for the update.

[jhump]

Resolved in #649, but only for unary-request RPCs (which includes both unary RPCs and server-streaming RPCs). Implementing GetBody for streaming requests is more complicated, and not as high of a priority since unary operations are the vast majority of RPC usage.

I've opened a separate issue (#672) to track the state of addressing this same issue with streaming RPCs.