Add Request.WithLimit

[gavv]

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)

[Rohith-Raju]

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

[gavv]

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: #245

[gavv]

I'd like to give this a go

Would you like to be assigned?

[Rohith-Raju]

Yes @gavv

[gavv]

Postponing this issue until #342 is merged. See #321 (comment)

[aliziyacevik]

Hey @gavv, I can take this one if it's available.

[gavv]

@aliziyacevik #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).

[gavv]

@aliziyacevik Hi, do you still have plans on the issue?