Skip to content

node: avoid double close resp.Body #31710

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 2, 2025
Merged

node: avoid double close resp.Body #31710

merged 1 commit into from
May 2, 2025

Conversation

@gzliudan
Copy link
Contributor

The functions rpcRequest and batchRpcRequest call baseRpcRequest. And resp.Body will be closed in the function baseRpcRequest later by t.Cleanup:

func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *http.Response {
        // ......
	t.Cleanup(func() { resp.Body.Close() })
	return resp
}

@gzliudan gzliudan requested a review from fjl as a code owner April 25, 2025 05:18
s1na
s1na approved these changes Apr 25, 2025
View reviewed changes
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@lightclient
Copy link
Member

Looks like Martin recommended using the t.Cleanup function to generally close the resp.Body across many uses, but there were a few from the original node refactor in #21105.

@lightclient lightclient merged commit 86a4924 into ethereum:master May 2, 2025
3 of 4 checks passed
@lightclient lightclient added this to the 1.15.11 milestone May 2, 2025
@gzliudan gzliudan deleted the close-resp-body-once branch May 2, 2025 15:42
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Jul 3, 2025
@gzliudan @jakub-freebit
node: avoid double close resp.Body (ethereum#31710)
bb0f40a 
The functions `rpcRequest` and `batchRpcRequest` call `baseRpcRequest`.
And `resp.Body` will be closed in the function `baseRpcRequest` later by
`t.Cleanup`:

```go
func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *http.Response {
        // ......
	t.Cleanup(func() { resp.Body.Close() })
	return resp
}
```
howjmay pushed a commit to iotaledger/go-ethereum that referenced this pull request Aug 27, 2025
@gzliudan @howjmay
node: avoid double close resp.Body (ethereum#31710)
bec5ef6 
The functions `rpcRequest` and `batchRpcRequest` call `baseRpcRequest`.
And `resp.Body` will be closed in the function `baseRpcRequest` later by
`t.Cleanup`:

```go
func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *http.Response {
        // ......
	t.Cleanup(func() { resp.Body.Close() })
	return resp
}
```
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
@gzliudan @gballet
node: avoid double close resp.Body (ethereum#31710)
c227b5f 
The functions `rpcRequest` and `batchRpcRequest` call `baseRpcRequest`.
And `resp.Body` will be closed in the function `baseRpcRequest` later by
`t.Cleanup`:

```go
func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *http.Response {
        // ......
	t.Cleanup(func() { resp.Body.Close() })
	return resp
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s1na s1na s1na approved these changes

@lightclient lightclient lightclient approved these changes

@fjl fjl Awaiting requested review from fjl fjl is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.15.11

Development

Successfully merging this pull request may close these issues.

3 participants

@gzliudan @lightclient @s1na