gavv
Add Request.WithLimit
Add new method Request.WithLimit that set's a limit for maximum number of bytes that can be retrieved from response body.
It can be helpful to interrupt early on abnormally large response (e.g. mistakenly trying to read infinite chunked response into memory), thus avoiding insane memory consumption by tests, swapping, etc.
Request.WithLimit should store the provided limit into a private field of Request. Later, in Request.retryRequest we should check if limit is set, and if so, pass this limit to bodyWrapper when we're wrapping response body.
Then, we should teach bodyWrapper to handle the limit. When the limit is set and bodyWrapper detected that it read more bytes than desired, it should abort read and return error that explains that limit is exceed and what is the limit.
Ideally error message should contain limit both in bytes and in human readable form, like "5 MB".
New method should have documentation comment with code example and two tests:
- unit test for request (request_test.go)
- end-to-end (blackbox) test (we can create new file e2e_limit_test.go)
Hey @gavv, I was looking into this issue. We can use this MaxBytesReader. Here's an example of how it can be implemented
https://go.dev/play/p/BJKXGLnwiL5
Then, we should teach bodyWrapper to handle the limit. When the limit is set and bodyWrapper detected that it read more bytes than desired, it should abort read and return error that explains that limit is exceed and what is the limit.
Even if the response exceeds the max size, we can return data(resp) until the max and return error/warning after reading the response. Are we separating functionality where we just report an error when it exceeds or return the data and error?
I'd like to give this a go
Hi,
Even if the response exceeds the max size, we can return data(resp) until the max and return error/warning after reading the response.
This sounds good, I was thinking about the same behavior.
We can use this MaxBytesReader.
Using MaxBytesReader is nice idea.
On the other hand, there are two thoughts why I think it's better to implement the check manually in bodyWrapper:
-
we already have one hidden wrapper (not reflected in Body type), using two nested hidden wrapper will make it even more confusing; I think it would be nice to keep all "wrapping" stuff in one place
-
to produce a nice error when limit exceeded, we anyway should catch error from MaxBytesReader and convert it to our own; so what we really get from MaxBytesReader is only counting bytes, but it's actually easy to implement
BTW, there is another issue related to bodyWrapper: https://github.com/gavv/httpexpect/issues/245
Postponing this issue until #342 is merged. See https://github.com/gavv/httpexpect/pull/321#issuecomment-1479613614
@aliziyacevik https://github.com/gavv/httpexpect/pull/342 was just merged today, so yes, you're welcome.
In the new implementation, I think it should be enough to modify httpReadNext and httpReadFull methods (but please re-check).
