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

fix: get body size 0 (#300) #380

Merged
merged 1 commit into from
Nov 1, 2020
Merged

Conversation

shiguangyin
Copy link
Contributor

when the http status code is 307 or 308, the request.Body will be closed, after that the func GetBody will be called but the bodyBuf was closed, then we will get an error http: ContentLength=xxx with Body length 0

@shiguangyin
Copy link
Contributor Author

@codecov
Copy link

codecov bot commented Sep 27, 2020

Codecov Report

Merging #380 into master will decrease coverage by 0.69%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
- Coverage   96.20%   95.51%   -0.70%     
==========================================
  Files          10       10              
  Lines        1238     1003     -235     
==========================================
- Hits         1191      958     -233     
+ Misses         26       25       -1     
+ Partials       21       20       -1     
Impacted Files Coverage Δ
client.go 95.78% <ø> (-0.95%) ⬇️
middleware.go 91.42% <100.00%> (-0.12%) ⬇️
redirect.go 92.00% <0.00%> (-2.12%) ⬇️
util.go 91.93% <0.00%> (-2.01%) ⬇️
retry.go 97.01% <0.00%> (-0.36%) ⬇️
resty.go 100.00% <0.00%> (ø)
trace.go 100.00% <0.00%> (ø)
request.go 100.00% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 608c8d7...b890aa0. Read the comment docs.

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.

@shiguangyin Thanks for the PR to fix the body size issue on redirects.

Can you please improve unit test cases for missed lines? you can see the code coverage report here #380 (comment)

@shiguangyin
Copy link
Contributor Author

@jeevatkm I think the code coverage report is not correct, I don't know why the code coverage decreased, I did't have any change on some files like redirect.go and util.go, but the report show those files are impacted. I don't know how to improve the unit test cases to increase the code coverage

@jeevatkm jeevatkm merged commit 0de0d7e into go-resty:master Nov 1, 2020
@jeevatkm jeevatkm added this to the v2.4.0 Milestone milestone Nov 1, 2020
@jeevatkm jeevatkm mentioned this pull request Nov 1, 2020
@jeevatkm jeevatkm added the bug label Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants