Skip to content

Conversation

@chkno
Copy link

@chkno chkno commented Mar 10, 2023

Add/improve some tests, especially around message size limits.

After reading the documentation, and the tests, I wasn't clear about what happens when one tries to read a message larger than ReadLimit, so I added a test for it:

  • so future users can see this at a glance by reading the tests, and
  • to emphasize that this behavior, down to the exact "Message exceeded max byte size" error message text, is part of the client API: Matching this error message text is the only way for clients to know that they need to retry their read with a larger ReadLimit. I find that I need to match this error text to detect this condition in a thing I'm writing that uses commitlog, and I'd feel better if the specific error text was guarded by a test.
  • to possibly start a discussion about whether clients ought to be depending on error message text like this / if there should be a better way to signal this 'you need to retry with a larger ReadLimit' condition, (or maybe even a way for a client to know how big the ReadLimit needs to be to read the next message without continually retrying with slightly higher limits). :)

chkno added 4 commits March 9, 2023 13:28
This is a critical part of the read API.  Because the default message
size write limit (1mb) is larger than the default read size (8kb),
clients using the default limits can write messages they cannot read.
Clients needs to be able to distinguish this message-too-big-to-read
case from other errors so they can retry the read with a larger ReadLimit.
@zowens
Copy link
Owner

zowens commented Mar 14, 2023

@chkno A more specific error message does make sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants