ttrpc icon indicating copy to clipboard operation
ttrpc copied to clipboard

channel: reject oversized messages on the sender side(, too).

Open klihub opened this issue 1 year ago • 2 comments

Reject oversized messages on the sender side (keeping the receiver side rejection code intact). Do not forcibly close in channel.send() the underlying connection when an oversized message is rejected.

This should provide minimal low-level means for ttrpc-based applications to detect oversized requests in client.Call() and attempt an application level recovery/corrective actions, if the high-level protocol is designed with this in mind.

klihub avatar Aug 21 '24 14:08 klihub

Uhmmkay... my intention was/is not to turn this into a Battle of the PRs (#119, #127 and this).

I simply ran into this message length limitation myself. But I did in a context where the higher-level protocol was fairly easy to update to allow sending 'multi-part' requests, which were then transparently recombined on the receiving end into a single one and 'handed that up in the communication stack' for processing. Also I did not have the danger of a response overflowing. But since I still considered hitting the size limit a corner case, I wrote an implementation which tries sending everything in a single request and falls back to splitting it up only if it hits the size limit. But for that I was missing a reliable way to detect an overflow, and do it in a way which does not render the underlying transport unusable for sending further messages (which is the case for #127, unconditionally). Hence I ended up rolling this.

Unfortunately I did not check if there are pending PRs trying to do the same. Once I noticed it, I tossed in Co-authored-by's for the other attempt's authors. I believe that this version does not suffer from the subject of the unaddressed review comments in the other PRs...

klihub avatar Aug 21 '24 16:08 klihub

@dmcgowan @fuweid Updated to implement the stock error Wrap interface instead of grpc's GRPCStatus() as discussed in the last community meeting.

klihub avatar Aug 24 '24 06:08 klihub

@fuweid @mikebrow PTAL

dmcgowan avatar Sep 26 '24 15:09 dmcgowan