Skip to content

Conversation

@jroper
Copy link
Contributor

@jroper jroper commented Dec 9, 2023

This fixes two bugs in outbound message size checking:

  • When the check failed, the thrown StatusRuntimeException with a status code of RESOURCE_EXHAUSTED was being caught and rewrapped in another StatusRuntimeException but this time with a status code of UNKNOWN, and the error message was lost.
  • This applies the max outbound message size check to messages prior to, and after compression, since compression of a message that is smaller than the maximum send size can result in a larger message that exceeds the maximum send size. See Should maxOutboundMessageSize be applied to compressed message bytes, as well as uncompressed message bytes? #10738.

Closes #10738.

This fixes two bugs in outbound message size checking:

* When thet checke failed, the thrown StatusRuntimeException with a status code
  of RESOURCE_EXHAUSTED was been caught and rewrapped in another
  StatusRuntimeException but this time with status code UNKNOWN.
* This applies the max outbound message size check to messages prior to, and
  after compression, since compression of a message that is smaller than the
  maximum send size can result in a larger message that exceeds the maximum
  send size.
@ejona86
Copy link
Member

ejona86 commented Dec 13, 2023

rewrapped in another StatusRuntimeException but this time with a status code of UNKNOWN

It should have been INTERNAL and the original exception in getCause()/causal chain. But yeah, better not to re-wrap it.

We really don't care that much about this level of precision of the outbound maximum message size, but that just means we wouldn't spend much effort on it. This is a very clean change, so easy to accept.

@ejona86 ejona86 requested a review from YifeiZhuang December 13, 2023 15:28
@ejona86
Copy link
Member

ejona86 commented Dec 13, 2023

This fixes #10738

@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 13, 2023
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 13, 2023
Copy link
Member

@YifeiZhuang YifeiZhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@YifeiZhuang YifeiZhuang merged commit 67b67f8 into grpc:master Dec 18, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should maxOutboundMessageSize be applied to compressed message bytes, as well as uncompressed message bytes?

4 participants