Why is payload_too_large? and the actual size of the payload not public? about grocer HOT 8 OPEN

A similar topic was brought up not too long ago in #78, but it was determined it we didn't need to expose the actual #encoded_payload. In your case, I can understand to expose the #payload_size while keeping the actual encoded payload an implementation detail.

An alternative idea: What if we enhance the #validate_payload to raise a PayloadTooLargeError error which has more details about the exact size. So you could directly use that if you wanted. And we could go a step further and also have the #valid? method shove the rescued exception's #message into an array of errors, which we expose via a #errors method. Thoughts?

from grocer.

I'd rather avoid using exceptions for flow control. If there's no particular reason for hiding these methods, I would simply expose them.

Not exposing encoded_payload is fine though.

Using #errors could also work, but it seems like too much trouble and indirect.

from grocer.

I'd rather avoid using exceptions for flow control.

I agree completely, which is why Grocer would expose a #valid? method for you to use. Internally we'd leverage exceptions, but we're not switching behavior based on them. And internally we do consider a notification with too large a body to be the exceptional case, rather than the expected case. But again, from the consumer's (i.e., your code's) perspective, it's just a predicate method, as per usual. ;)

I was thinking the #valid? + #errors would be nice b/c it would prevent consumers from having to know all of the various ways that something might be invalid. (e.g., no payload, payload too large, etc.)

from grocer.

+1 for #valid? and #errors, but how do you get the bytesize from #errors?

from grocer.

@n-studio I simply made payload_size public

from grocer.

@n-studio My thinking is: as part of the enhancement to #valid? and exposing #errors, we'd put the current encoded_payload.bytesize into the error message we give to PayloadTooLargeError. Does that make sense?

from grocer.

@mbillard I do still think going with #valid and #errors to be a great option for handling this, but I don't have the need for it now. However, you do need to know that information. If you're up for submitting your changes as a PR, I think we can merge them.

Thoughts?

from grocer.

@mbillard Yes, I think it's necessary to have payload_size public.

@stevenharman If encoded_payload.bytesize is only in the error message then we need to parse the message to get the size, which would be ugly.
But if payload_size is public, everything is great!

from grocer.