rpc: Avoid unnecessary RST_STREAM and PING frames causing GOAWAYs

Author: michaelkaplan13

beacon/light/api/light_api.go

@@ -35,6 +35,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/rpc"
 )
 
 var (
@@ -137,7 +138,7 @@ func (api *BeaconLightApi) httpGet(path string, params url.Values) ([]byte, erro
 	if err != nil {
 		return nil, err
 	}
-	defer resp.Body.Close()
+	defer rpc.CleanlyCloseBody(resp.Body)
 	switch resp.StatusCode {
 	case 200:
 		return io.ReadAll(resp.Body)

cmd/geth/version_check.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/jedisct1/go-minisign"
 	"github.com/urfave/cli/v2"
 )
@@ -119,7 +120,7 @@ func fetch(url string) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer res.Body.Close()
+	defer rpc.CleanlyCloseBody(res.Body)
 	body, err := io.ReadAll(res.Body)
 	if err != nil {
 		return nil, err

graphql/graphql_test.go

@@ -40,6 +40,7 @@ import (
 	"github.com/ethereum/go-ethereum/eth/filters"
 	"github.com/ethereum/go-ethereum/node"
 	"github.com/ethereum/go-ethereum/params"
+	"github.com/ethereum/go-ethereum/rpc"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -159,7 +160,7 @@ func TestGraphQLBlockSerialization(t *testing.T) {
 			t.Fatalf("could not post: %v", err)
 		}
 		bodyBytes, err := io.ReadAll(resp.Body)
-		resp.Body.Close()
+		rpc.CleanlyCloseBody(resp.Body)
 		if err != nil {
 			t.Fatalf("could not read from response body: %v", err)
 		}
@@ -246,7 +247,7 @@ func TestGraphQLBlockSerializationEIP2718(t *testing.T) {
 			t.Fatalf("could not post: %v", err)
 		}
 		bodyBytes, err := io.ReadAll(resp.Body)
-		resp.Body.Close()
+		rpc.CleanlyCloseBody(resp.Body)
 		if err != nil {
 			t.Fatalf("could not read from response body: %v", err)
 		}
@@ -271,7 +272,7 @@ func TestGraphQLHTTPOnSamePort_GQLRequest_Unsuccessful(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not post: %v", err)
 	}
-	resp.Body.Close()
+	rpc.CleanlyCloseBody(resp.Body)
 	// make sure the request is not handled successfully
 	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
 }

internal/download/download.go

@@ -30,6 +30,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/ethereum/go-ethereum/rpc"
 )
 
 // ChecksumDB keeps file checksums and tool versions.
@@ -191,7 +193,7 @@ func (db *ChecksumDB) DownloadFile(url, dstPath string) error {
 	if err != nil {
 		return fmt.Errorf("download error: %v", err)
 	}
-	defer resp.Body.Close()
+	defer rpc.CleanlyCloseBody(resp.Body)
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("download error: status %d", resp.StatusCode)
 	}

metrics/influxdb/influxdb_test.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/metrics"
 	"github.com/ethereum/go-ethereum/metrics/internal"
+	"github.com/ethereum/go-ethereum/rpc"
 	influxdb2 "github.com/influxdata/influxdb-client-go/v2"
 )
 
@@ -47,7 +48,7 @@ func TestExampleV1(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		haveB, _ := io.ReadAll(r.Body)
 		have = string(haveB)
-		r.Body.Close()
+		rpc.CleanlyCloseBody(r.Body)
 	}))
 	defer ts.Close()
 	u, _ := url.Parse(ts.URL)
@@ -83,7 +84,7 @@ func TestExampleV2(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		haveB, _ := io.ReadAll(r.Body)
 		have = string(haveB)
-		r.Body.Close()
+		rpc.CleanlyCloseBody(r.Body)
 	}))
 	defer ts.Close()
 

node/api_test.go

@@ -318,7 +318,7 @@ func checkBodyOK(url string) bool {
 	if err != nil {
 		return false
 	}
-	defer resp.Body.Close()
+	rpc.CleanlyCloseBody(resp.Body)
 
 	if resp.StatusCode != 200 {
 		return false

node/node_test.go

@@ -600,7 +600,7 @@ func doHTTPRequest(t *testing.T, req *http.Request) *http.Response {
 	if err != nil {
 		t.Fatalf("could not issue a GET request to the given endpoint: %v", err)
 	}
-	t.Cleanup(func() { resp.Body.Close() })
+	t.Cleanup(func() { rpc.CleanlyCloseBody(resp.Body) })
 	return resp
 }
 

node/rpcstack_test.go

@@ -320,7 +320,7 @@ func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *
 	if err != nil {
 		t.Fatal(err)
 	}
-	t.Cleanup(func() { resp.Body.Close() })
+	t.Cleanup(func() { rpc.CleanlyCloseBody(resp.Body) })
 	return resp
 }
 
@@ -530,7 +530,7 @@ func TestGzipHandler(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			defer resp.Body.Close()
+			defer rpc.CleanlyCloseBody(resp.Body)
 
 			content, err := io.ReadAll(resp.Body)
 			if err != nil {

rpc/http.go

@@ -168,13 +168,21 @@ func newClientTransportHTTP(endpoint string, cfg *clientConfig) reconnectFunc {
 	}
 }
 
+// CleanlyCloseBody avoids sending unnecessary RST_STREAM and PING frames by ensuring
+// the whole body is read before being closed.
+// See https://blog.cloudflare.com/go-and-enhance-your-calm/#reading-bodies-in-go-can-be-unintuitive
+func CleanlyCloseBody(body io.ReadCloser) error {
+	io.Copy(io.Discard, body)
+	return body.Close()
+}
+
 func (c *Client) sendHTTP(ctx context.Context, op *requestOp, msg interface{}) error {
 	hc := c.writeConn.(*httpConn)
 	respBody, err := hc.doRequest(ctx, msg)
 	if err != nil {
 		return err
 	}
-	defer respBody.Close()
+	defer CleanlyCloseBody(respBody)
 
 	var resp jsonrpcMessage
 	batch := [1]*jsonrpcMessage{&resp}
@@ -191,7 +199,7 @@ func (c *Client) sendBatchHTTP(ctx context.Context, op *requestOp, msgs []*jsonr
 	if err != nil {
 		return err
 	}
-	defer respBody.Close()
+	defer CleanlyCloseBody(respBody)
 
 	var respmsgs []*jsonrpcMessage
 	if err := json.NewDecoder(respBody).Decode(&respmsgs); err != nil {
@@ -236,7 +244,7 @@ func (hc *httpConn) doRequest(ctx context.Context, msg interface{}) (io.ReadClos
 		if _, err := buf.ReadFrom(resp.Body); err == nil {
 			body = buf.Bytes()
 		}
-		resp.Body.Close()
+		CleanlyCloseBody(resp.Body)
 		return nil, HTTPError{
 			Status:     resp.Status,
 			StatusCode: resp.StatusCode,

rpc/http_test.go

@@ -106,7 +106,7 @@ func confirmHTTPRequestYieldsStatusCode(t *testing.T, method, contentType, body
 	if err != nil {
 		t.Fatalf("request failed: %v", err)
 	}
-	resp.Body.Close()
+	CleanlyCloseBody(resp.Body)
 	confirmStatusCode(t, resp.StatusCode, expectedStatusCode)
 }