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

Improvements to HTTP response handling #14673

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
- Updated light client consensus types. [PR](https://github.com/prysmaticlabs/prysm/pull/14652)
- Fixed pending deposits processing on Electra.
- Modified `ListAttestationsV2`, `GetAttesterSlashingsV2` and `GetAggregateAttestationV2` endpoints to use slot to determine fork version.
- Improvements to HTTP response handling. [pr](https://github.com/prysmaticlabs/prysm/pull/14673)

### Deprecated

Expand Down
1 change: 1 addition & 0 deletions api/client/builder/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//api:go_default_library",
"//api/client:go_default_library",
"//api/server/structs:go_default_library",
"//config/fieldparams:go_default_library",
"//consensus-types:go_default_library",
Expand Down
5 changes: 3 additions & 2 deletions api/client/builder/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/api"
"github.com/prysmaticlabs/prysm/v5/api/client"
"github.com/prysmaticlabs/prysm/v5/api/server/structs"
"github.com/prysmaticlabs/prysm/v5/consensus-types/blocks"
"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
Expand Down Expand Up @@ -176,7 +177,7 @@ func (c *Client) do(ctx context.Context, method string, path string, body io.Rea
err = non200Err(r)
return
}
res, err = io.ReadAll(r.Body)
res, err = io.ReadAll(io.LimitReader(r.Body, client.MaxBodySize))
if err != nil {
err = errors.Wrap(err, "error reading http response body from builder server")
return
Expand Down Expand Up @@ -358,7 +359,7 @@ func (c *Client) Status(ctx context.Context) error {
}

func non200Err(response *http.Response) error {
bodyBytes, err := io.ReadAll(response.Body)
bodyBytes, err := io.ReadAll(io.LimitReader(response.Body, client.MaxErrBodySize))
var errMessage ErrorMessage
var body string
if err != nil {
Expand Down
21 changes: 14 additions & 7 deletions api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,17 @@ import (
"github.com/pkg/errors"
)

const (
MaxBodySize int64 = 1 << 23 // 8MB default, WithMaxBodySize can override
MaxErrBodySize int64 = 1 << 17 // 128KB
)

// Client is a wrapper object around the HTTP client.
type Client struct {
hc *http.Client
baseURL *url.URL
token string
hc *http.Client
baseURL *url.URL
token string
maxBodySize int64
}

// NewClient constructs a new client with the provided options (ex WithTimeout).
Expand All @@ -26,8 +32,9 @@ func NewClient(host string, opts ...ClientOpt) (*Client, error) {
return nil, err
}
c := &Client{
hc: &http.Client{},
baseURL: u,
hc: &http.Client{},
baseURL: u,
maxBodySize: MaxBodySize,
}
for _, o := range opts {
o(c)
Expand Down Expand Up @@ -72,7 +79,7 @@ func (c *Client) NodeURL() string {
// Get is a generic, opinionated GET function to reduce boilerplate amongst the getters in this package.
func (c *Client) Get(ctx context.Context, path string, opts ...ReqOption) ([]byte, error) {
u := c.baseURL.ResolveReference(&url.URL{Path: path})
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), http.NoBody)
if err != nil {
return nil, err
}
Expand All @@ -89,7 +96,7 @@ func (c *Client) Get(ctx context.Context, path string, opts ...ReqOption) ([]byt
if r.StatusCode != http.StatusOK {
return nil, Non200Err(r)
}
b, err := io.ReadAll(r.Body)
b, err := io.ReadAll(io.LimitReader(r.Body, c.maxBodySize))
if err != nil {
return nil, errors.Wrap(err, "error reading http response body")
}
Expand Down
10 changes: 5 additions & 5 deletions api/client/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ var ErrInvalidNodeVersion = errors.New("invalid node version response")
var ErrConnectionIssue = errors.New("could not connect")

// Non200Err is a function that parses an HTTP response to handle responses that are not 200 with a formatted error.
func Non200Err(response *http.Response) error {
bodyBytes, err := io.ReadAll(response.Body)
func Non200Err(r *http.Response) error {
b, err := io.ReadAll(io.LimitReader(r.Body, MaxErrBodySize))
var body string
if err != nil {
body = "(Unable to read response body.)"
} else {
body = "response body:\n" + string(bodyBytes)
body = "response body:\n" + string(b)
}
msg := fmt.Sprintf("code=%d, url=%s, body=%s", response.StatusCode, response.Request.URL, body)
switch response.StatusCode {
msg := fmt.Sprintf("code=%d, url=%s, body=%s", r.StatusCode, r.Request.URL, body)
switch r.StatusCode {
case http.StatusNotFound:
return errors.Wrap(ErrNotFound, msg)
default:
Expand Down
7 changes: 7 additions & 0 deletions api/client/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,10 @@ func WithAuthenticationToken(token string) ClientOpt {
c.token = token
}
}

// WithMaxBodySize overrides the default max body size of 8MB.
func WithMaxBodySize(size int64) ClientOpt {
return func(c *Client) {
c.maxBodySize = size
}
}
1 change: 1 addition & 0 deletions beacon-chain/sync/checkpoint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ go_library(
importpath = "github.com/prysmaticlabs/prysm/v5/beacon-chain/sync/checkpoint",
visibility = ["//visibility:public"],
deps = [
"//api/client:go_default_library",
"//api/client/beacon:go_default_library",
"//beacon-chain/db:go_default_library",
"//config/params:go_default_library",
Expand Down
12 changes: 7 additions & 5 deletions beacon-chain/sync/checkpoint/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import (
"context"

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/api/client"
"github.com/prysmaticlabs/prysm/v5/api/client/beacon"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/db"
"github.com/prysmaticlabs/prysm/v5/config/params"
)

const stateSizeLimit int64 = 1 << 29 // 512MB

// APIInitializer manages initializing the beacon node using checkpoint sync, retrieving the checkpoint state and root
// from the remote beacon node api.
type APIInitializer struct {
Expand All @@ -18,7 +21,7 @@ type APIInitializer struct {
// NewAPIInitializer creates an APIInitializer, handling the set up of a beacon node api client
// using the provided host string.
func NewAPIInitializer(beaconNodeHost string) (*APIInitializer, error) {
c, err := beacon.NewClient(beaconNodeHost)
c, err := beacon.NewClient(beaconNodeHost, client.WithMaxBodySize(stateSizeLimit))
if err != nil {
return nil, errors.Wrapf(err, "unable to parse beacon node url or hostname - %s", beaconNodeHost)
}
Expand All @@ -32,10 +35,9 @@ func (dl *APIInitializer) Initialize(ctx context.Context, d db.Database) error {
if err == nil && origin != params.BeaconConfig().ZeroHash {
log.Warnf("Origin checkpoint root %#x found in db, ignoring checkpoint sync flags", origin)
return nil
} else {
if !errors.Is(err, db.ErrNotFound) {
return errors.Wrap(err, "error while checking database for origin root")
}
}
if err != nil && !errors.Is(err, db.ErrNotFound) {
return errors.Wrap(err, "error while checking database for origin root")
}
od, err := beacon.DownloadFinalizedData(ctx, dl.c)
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion cmd/prysmctl/checkpointsync/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@ var downloadCmd = &cli.Command{
},
}

const stateSizeLimit int64 = 1 << 29 // 512MB to accommodate future state growth

func cliActionDownload(_ *cli.Context) error {
ctx := context.Background()
f := downloadFlags

opts := []client.ClientOpt{client.WithTimeout(f.Timeout)}
opts := []client.ClientOpt{client.WithTimeout(f.Timeout), client.WithMaxBodySize(stateSizeLimit)}
client, err := beacon.NewClient(downloadFlags.BeaconNodeHost, opts...)
if err != nil {
return err
Expand Down
Loading