Improve error reporting on oversized messages #572
Conversation
| @@ -31,7 +31,7 @@ namespace asynchost | |||
| friend class close_ptr<TCPImpl>; | |||
|
|
|||
| static constexpr int backlog = 128; | |||
| static constexpr size_t read_size = 1024; | |||
| static constexpr size_t max_read_size = 16384; | |||
There was a problem hiding this comment.
This reduces the output spam for large messages (reduces the number of reads by a factor of 16 so we), and should also improve performance for them. But this may be at the cost of performance for smaller messages, and was a deadend in tracking down the actual failure so should perhaps be removed.
There was a problem hiding this comment.
Should this be a comment in the code as well?
There was a problem hiding this comment.
It seems like this doesn't actually have a noticeable impact on performance either way, so I think we can just ignore this for now (and hopefully remember it as a potentially bad choice if it comes back to bite us in future?).
I think it should actually be possible to just keep a single buffer in this object, rather than reallocating on every request, but I'll also leave that investigation on the backburner atm.
|
|
|
I haven't reproduced this yet, but we can also hit this size limit for node-to-node communications via |
Codecov Report
@@ Coverage Diff @@
## master #572 +/- ##
==========================================
- Coverage 78.56% 78.34% -0.22%
==========================================
Files 140 140
Lines 10567 10599 +32
==========================================
+ Hits 8301 8303 +2
- Misses 2266 2296 +30
|
|
|
* Add errors for oversized messages * Add trace of message sizes * Restore previous max msg size * Increase max TCP read size
tlsframedendpointimposes a maximum message size of 2MB, but previously did so by closing connections with no logging. I've tried to improve this by logging a failure when we close these connections, logging more clearly when we're ignoring messages for closed connections, and trying to send a reasonable message back over the connection before we close it.I made the
e2e_loggingtests deliberately increase message sizes until it hit this error, but have removed as this didn't work - the error is reported for seqno 0 so we need to fetch from the stream manually, and still we sometimes get a closed session without any preceding error message.