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
fix(pubsub): ignore grpc errors in ack/modack #5796
Conversation
// This addresses an error where `context deadline exceeded` errors | ||
// not captured by the previous case causes fatal errors. | ||
// See https://github.com/googleapis/google-cloud-go/issues/3060 | ||
if strings.Contains(err.Error(), "context deadline exceeded") { | ||
if err != nil && strings.Contains(err.Error(), "context deadline exceeded") { |
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.
Does the RPC error include an ErrorInfo? We should really be using that rather than relying on the error message.
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.
So this was a strange error that wasn't coming from the server (as far as I could tell) so I'm not entirely sure if it has ErrorInfo
. I believe the real fix for this is to find the underlying cause where this context deadline exceeded
is bubbling up and address it there (might be coming from the grpc-go library).
With the upcoming exactly-once delivery feature, acks/modacks can return errors. However, if the user is not using exactly-once, these errors should be ignored as ack/modack should be fire-and-forget. When exactly-once is supported, we will also need to check if the user has set exactly-once from StreamingPull responses and retry or return the error.
Fixes #5797