Avoid unnecessary RST_STREAM and PING frames causing GOAWAY messages (#4480)

Co-authored-by: Stephen Buttolph <[email protected]>

Author: michaelkaplan13

api/metrics/client.go

@@ -12,6 +12,8 @@ import (
 
 	"github.com/prometheus/common/expfmt"
 
+	"github.com/ava-labs/avalanchego/utils/rpc"
+
 	dto "github.com/prometheus/client_model/go"
 )
 
@@ -45,24 +47,18 @@ func (c *Client) GetMetrics(ctx context.Context) (map[string]*dto.MetricFamily,
 		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
 
+	//nolint:bodyclose // body is closed via rpc.CleanlyCloseBody
 	resp, err := http.DefaultClient.Do(request)
 	if err != nil {
 		return nil, fmt.Errorf("failed to issue request: %w", err)
 	}
+	defer rpc.CleanlyCloseBody(resp.Body)
 
 	// Return an error for any non successful status code
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		// Drop any error during close to report the original error
-		_ = resp.Body.Close()
 		return nil, fmt.Errorf("received status code: %d", resp.StatusCode)
 	}
 
 	var parser expfmt.TextParser
-	metrics, err := parser.TextToMetricFamilies(resp.Body)
-	if err != nil {
-		// Drop any error during close to report the original error
-		_ = resp.Body.Close()
-		return nil, err
-	}
-	return metrics, resp.Body.Close()
+	return parser.TextToMetricFamilies(resp.Body)
 }

tests/fixture/tmpnet/check_monitoring.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/ava-labs/avalanchego/tests/fixture/stacktrace"
 	"github.com/ava-labs/avalanchego/utils/logging"
+	"github.com/ava-labs/avalanchego/utils/rpc"
 )
 
 type getCountFunc func() (int, error)
@@ -110,11 +111,12 @@ func queryLoki(
 	req.Header.Set("Authorization", "Basic "+auth)
 
 	// Execute request
+	//nolint:bodyclose // body is closed via rpc.CleanlyCloseBody
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return 0, stacktrace.Errorf("failed to execute request: %w", err)
 	}
-	defer resp.Body.Close()
+	defer rpc.CleanlyCloseBody(resp.Body)
 
 	// Read and parse response
 	body, err := io.ReadAll(resp.Body)

tests/fixture/tmpnet/monitor_processes.go

@@ -24,6 +24,7 @@ import (
 	"github.com/ava-labs/avalanchego/tests/fixture/stacktrace"
 	"github.com/ava-labs/avalanchego/utils/logging"
 	"github.com/ava-labs/avalanchego/utils/perms"
+	"github.com/ava-labs/avalanchego/utils/rpc"
 )
 
 const (
@@ -587,11 +588,12 @@ func checkReadiness(ctx context.Context, url string) (bool, string, error) {
 		return false, "", stacktrace.Wrap(err)
 	}
 
+	//nolint:bodyclose // body is closed via rpc.CleanlyCloseBody
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return false, "", stacktrace.Errorf("request failed: %w", err)
 	}
-	defer resp.Body.Close()
+	defer rpc.CleanlyCloseBody(resp.Body)
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {

tests/fixture/tmpnet/node.go

@@ -24,6 +24,7 @@ import (
 	"github.com/ava-labs/avalanchego/tests/fixture/stacktrace"
 	"github.com/ava-labs/avalanchego/utils/constants"
 	"github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner"
+	"github.com/ava-labs/avalanchego/utils/rpc"
 	"github.com/ava-labs/avalanchego/vms/platformvm/signer"
 )
 
@@ -220,11 +221,13 @@ func (n *Node) SaveMetricsSnapshot(ctx context.Context) error {
 	if err != nil {
 		return stacktrace.Wrap(err)
 	}
+	//nolint:bodyclose // body is closed via rpc.CleanlyCloseBody
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return stacktrace.Wrap(err)
 	}
-	defer resp.Body.Close()
+	defer rpc.CleanlyCloseBody(resp.Body)
+
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return stacktrace.Wrap(err)

utils/dynamicip/ifconfig_resolver.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"github.com/ava-labs/avalanchego/utils/ips"
+	"github.com/ava-labs/avalanchego/utils/rpc"
 )
 
 var _ Resolver = (*ifConfigResolver)(nil)
@@ -27,11 +28,12 @@ func (r *ifConfigResolver) Resolve(ctx context.Context) (netip.Addr, error) {
 		return netip.Addr{}, err
 	}
 
+	//nolint:bodyclose // body is closed via rpc.CleanlyCloseBody
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return netip.Addr{}, err
 	}
-	defer resp.Body.Close()
+	defer rpc.CleanlyCloseBody(resp.Body)
 
 	ipBytes, err := io.ReadAll(resp.Body)
 	if err != nil {

utils/rpc/json.go

@@ -7,12 +7,21 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 
 	rpc "github.com/gorilla/rpc/v2/json2"
 )
 
+// 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) {
+	_, _ = io.Copy(io.Discard, body)
+	_ = body.Close()
+}
+
 func SendJSONRequest(
 	ctx context.Context,
 	uri *url.URL,
@@ -42,22 +51,21 @@ func SendJSONRequest(
 	request.Header = ops.headers
 	request.Header.Set("Content-Type", "application/json")
 
+	//nolint:bodyclose // body is closed via CleanlyCloseBody
 	resp, err := http.DefaultClient.Do(request)
 	if err != nil {
 		return fmt.Errorf("failed to issue request: %w", err)
 	}
+	defer CleanlyCloseBody(resp.Body)
 
 	// Return an error for any non successful status code
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		// Drop any error during close to report the original error
-		_ = resp.Body.Close()
 		return fmt.Errorf("received status code: %d", resp.StatusCode)
 	}
 
 	if err := rpc.DecodeClientResponse(resp.Body, reply); err != nil {
-		// Drop any error during close to report the original error
-		_ = resp.Body.Close()
 		return fmt.Errorf("failed to decode client response: %w", err)
 	}
-	return resp.Body.Close()
+
+	return nil
 }