fix: close response Body when the returned error is nil #146
Conversation
| if resp.Body != nil { | ||
| _ = resp.Body.Close() | ||
| } | ||
| _ = resp.Body.Close() |
There was a problem hiding this comment.
Why was the resp.Body nil check removed here?
There was a problem hiding this comment.
According to this official http client documentation, If the returned error is nil, the Response will contain a non-nil Body
There was a problem hiding this comment.
If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close. If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport) may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.
On error, any Response can be ignored. A non-nil Response with a non-nil error only occurs when CheckRedirect fails, and even then the returned Response.Body is already closed.
According to the above, on success, we need to close the Body explicitly. On error, the body is already closed.
As long as the double close doesn't result in a panic(), I am ok with this.
| @@ -370,15 +370,14 @@ func (c *Client) getAllKeys(subPath string) (map[string]string, error) { | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| defer func() { | |||
| _ = resp.Body.Close() | |||
There was a problem hiding this comment.
This code is not checking that resp.Body is nil or not before calling Close
|
@gao270615179 You need to sign your commit before it can be merged. Please sign the commit (commit --amend -s and maybe -S) and force-push. |
Signed-off-by: gao270615179 <[email protected]>
Signed-off-by: gao270615179 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #146 +/- ##
==========================================
+ Coverage 77.66% 77.88% +0.22%
==========================================
Files 17 17
Lines 900 900
==========================================
+ Hits 699 701 +2
+ Misses 143 142 -1
+ Partials 58 57 -1
Continue to review full report at Codecov.
|
I have signed the commit and fixed secrets_test InMemoryMockCaller response. |
* fix: close response Body when the returned error is nil Signed-off-by: gao270615179 <[email protected]> * fix: add InMemoryMockCaller response body Signed-off-by: gao270615179 <[email protected]> Co-authored-by: gao270615179 <[email protected]>
Issue: #145
According to this official http client documentation,
If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close.. The Response Body must be closed when the returned error is nil. And we don't need check whether the Response Body is nil.If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-secrets/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:describing the break)Testing Instructions
New Dependency Instructions (If applicable)