Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add max reponse body limit #830

Merged
merged 13 commits into from
Aug 29, 2024
Merged

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented Aug 27, 2024

Client not will be able to check resposne body size limit.

If server return a large response than expected, it will discard the response and return a err ErrResponseBodyTooLarge.

c := Client().SetResponseBodyLimit(1024*1024*1024) // do not allow body larger than 1GiB
res, err := c.R().Get(...)
if err != nil{
  if errors.Is(err, resty.ErrResponseBodyTooLarge){
    ...
  }
}

close #819

@trim21 trim21 changed the title feat: add max reponse body limit for uncompressed data feat: add max reponse body limit Aug 27, 2024
@trim21 trim21 marked this pull request as ready for review August 27, 2024 23:37
@trim21 trim21 mentioned this pull request Aug 27, 2024
@jeevatkm
Copy link
Member

@trim21 Thanks for creating a PR.

I wanted to confirm before I start reviewing it. If you don't mind, your use case is to fail the request if the response body size exceeds the specific size. Can you update the PR description to one use case and its example?

@trim21
Copy link
Contributor Author

trim21 commented Aug 28, 2024

@trim21 Thanks for creating a PR.

I wanted to confirm before I start reviewing it. If you don't mind, your use case is to fail the request if the response body size exceeds the specific size. Can you update the PR description to one use case and its example?

is this ok?

Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trim21 Thanks for adding the PR description with details.

I have shared my PR review comments.

client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
@jeevatkm jeevatkm added this to the v2.15.0 milestone Aug 28, 2024
@trim21
Copy link
Contributor Author

trim21 commented Aug 28, 2024

I'm not sure size limit should be int or int64.

It's a length limit of []byte, and len([]byte) is int, so it would make more sense to be int?

@jeevatkm
Copy link
Member

I'm not sure size limit should be int or int64.

It's a length limit of []byte, and len([]byte) is int, so it would make more sense to be int?

@trim21 Yes, for size limit, keep type int

jeevatkm
jeevatkm previously approved these changes Aug 28, 2024
Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trim21 Thanks for updating the PR.

Copy link

codecov bot commented Aug 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.89%. Comparing base (f575bf6) to head (663fdae).
Report is 1 commits behind head on v2.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2     #830      +/-   ##
==========================================
+ Coverage   96.68%   96.89%   +0.21%     
==========================================
  Files          14       14              
  Lines        1748     1773      +25     
==========================================
+ Hits         1690     1718      +28     
+ Misses         37       35       -2     
+ Partials       21       20       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jeevatkm
Copy link
Member

@trim21 Can you please check this https:/go-resty/resty/pull/830/checks?check_run_id=29386261232 and improve it?

@trim21

This comment was marked as outdated.

Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trim21 Thanks for the test coverage updates. Now all green.

@jeevatkm jeevatkm merged commit 10bf84f into go-resty:v2 Aug 29, 2024
3 checks passed
@trim21 trim21 deleted the limit-res-body-size branch August 29, 2024 17:49
renovate bot referenced this pull request in Michsior14/transmission-gluetun-port-update Sep 15, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/go-resty/resty/v2](https://redirect.github.com/go-resty/resty)
| `v2.14.0` -> `v2.15.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgo-resty%2fresty%2fv2/v2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgo-resty%2fresty%2fv2/v2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgo-resty%2fresty%2fv2/v2.14.0/v2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgo-resty%2fresty%2fv2/v2.14.0/v2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>go-resty/resty (github.com/go-resty/resty/v2)</summary>

###
[`v2.15.0`](https://redirect.github.com/go-resty/resty/releases/tag/v2.15.0)

[Compare
Source](https://redirect.github.com/go-resty/resty/compare/v2.14.0...v2.15.0)

### Release Notes

#### New Features

- feat: add max reponse body limit by
[@&#8203;trim21](https://redirect.github.com/trim21) in
[https:/go-resty/resty/pull/830](https://redirect.github.com/go-resty/resty/pull/830)
- feat: add SetClientRootCertificate method support clientCAs usage by
[@&#8203;MagHErmit](https://redirect.github.com/MagHErmit) in
[https:/go-resty/resty/pull/826](https://redirect.github.com/go-resty/resty/pull/826)

#### Enhancements

- feat: add ability to set custom multipart boundary value by
[@&#8203;PokeGuys](https://redirect.github.com/PokeGuys) in
[https:/go-resty/resty/pull/820](https://redirect.github.com/go-resty/resty/pull/820)
- feat(refactor): for pr
[#&#8203;826](https://redirect.github.com/go-resty/resty/issues/826) by
[@&#8203;jeevatkm](https://redirect.github.com/jeevatkm) in
[https:/go-resty/resty/pull/848](https://redirect.github.com/go-resty/resty/pull/848)

#### Bug Fixes

- Fix request/response logging for SetDoNotParseResponse(true) by
[@&#8203;kon3gor](https://redirect.github.com/kon3gor) in
[https:/go-resty/resty/pull/836](https://redirect.github.com/go-resty/resty/pull/836)
- fix(enhancement): add explicit option to enable generate curl command
in conjunction with debug mode and few clean ups
[#&#8203;828](https://redirect.github.com/go-resty/resty/issues/828) by
[@&#8203;jeevatkm](https://redirect.github.com/jeevatkm) in
[https:/go-resty/resty/pull/842](https://redirect.github.com/go-resty/resty/pull/842)

#### Build

- feat(enhancement): update bazel config by
[@&#8203;frank30941](https://redirect.github.com/frank30941) in
[https:/go-resty/resty/pull/833](https://redirect.github.com/go-resty/resty/pull/833)
- build: go min version and build config update
[#&#8203;835](https://redirect.github.com/go-resty/resty/issues/835) by
[@&#8203;jeevatkm](https://redirect.github.com/jeevatkm) in
[https:/go-resty/resty/pull/837](https://redirect.github.com/go-resty/resty/pull/837)
- Update go vesion to 1.20 by
[@&#8203;kon3gor](https://redirect.github.com/kon3gor) in
[https:/go-resty/resty/pull/841](https://redirect.github.com/go-resty/resty/pull/841)

#### Documentation

- doc: godoc improvements and corrections by
[@&#8203;jeevatkm](https://redirect.github.com/jeevatkm) in
[https:/go-resty/resty/pull/849](https://redirect.github.com/go-resty/resty/pull/849),
[https:/go-resty/resty/pull/851](https://redirect.github.com/go-resty/resty/pull/851)
- doc(readme): Add a note, GenerateCurlCommand by
[@&#8203;ahuigo](https://redirect.github.com/ahuigo) in
[https:/go-resty/resty/pull/817](https://redirect.github.com/go-resty/resty/pull/817)
- release: version bump and readme update for v2.15.0 by
[@&#8203;jeevatkm](https://redirect.github.com/jeevatkm) in
[https:/go-resty/resty/pull/852](https://redirect.github.com/go-resty/resty/pull/852)

#### New Contributors

- [@&#8203;PokeGuys](https://redirect.github.com/PokeGuys) made their
first contribution in
[https:/go-resty/resty/pull/820](https://redirect.github.com/go-resty/resty/pull/820)
- [@&#8203;trim21](https://redirect.github.com/trim21) made their first
contribution in
[https:/go-resty/resty/pull/830](https://redirect.github.com/go-resty/resty/pull/830)
- [@&#8203;frank30941](https://redirect.github.com/frank30941) made
their first contribution in
[https:/go-resty/resty/pull/833](https://redirect.github.com/go-resty/resty/pull/833)
- [@&#8203;kon3gor](https://redirect.github.com/kon3gor) made their
first contribution in
[https:/go-resty/resty/pull/841](https://redirect.github.com/go-resty/resty/pull/841)
- [@&#8203;MagHErmit](https://redirect.github.com/MagHErmit) made their
first contribution in
[https:/go-resty/resty/pull/826](https://redirect.github.com/go-resty/resty/pull/826)

**Full Changelog**:
go-resty/resty@v2.14.0...v2.15.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/Michsior14/transmission-gluetun-port-update).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC43NC4xIiwidXBkYXRlZEluVmVyIjoiMzguNzQuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

max reponse body limit
2 participants