From 29360f955c8cbce72fb666d75a90b342d2261e4a Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 18 Feb 2021 23:49:24 +0100 Subject: [PATCH] light: improve timeout functionality (#6145) --- cmd/tendermint/commands/light.go | 10 +++--- light/provider/http/http.go | 49 +++++++++++++------------- light/rpc/client.go | 2 +- rpc/jsonrpc/client/http_json_client.go | 5 +-- 4 files changed, 32 insertions(+), 34 deletions(-) diff --git a/cmd/tendermint/commands/light.go b/cmd/tendermint/commands/light.go index b9e3741c5..1bcce8f5b 100644 --- a/cmd/tendermint/commands/light.go +++ b/cmd/tendermint/commands/light.go @@ -206,11 +206,6 @@ func runProxy(cmd *cobra.Command, args []string) error { return err } - rpcClient, err := rpchttp.New(primaryAddr, "/websocket") - if err != nil { - return fmt.Errorf("http client for %s: %w", primaryAddr, err) - } - cfg := rpcserver.DefaultConfig() cfg.MaxBodyBytes = config.RPC.MaxBodyBytes cfg.MaxHeaderBytes = config.RPC.MaxHeaderBytes @@ -222,6 +217,11 @@ func runProxy(cmd *cobra.Command, args []string) error { cfg.WriteTimeout = config.RPC.TimeoutBroadcastTxCommit + 1*time.Second } + rpcClient, err := rpchttp.NewWithTimeout(primaryAddr, "/websocket", cfg.WriteTimeout) + if err != nil { + return fmt.Errorf("failed to create http client for %s: %w", primaryAddr, err) + } + p := lproxy.Proxy{ Addr: listenAddr, Config: cfg, diff --git a/light/provider/http/http.go b/light/provider/http/http.go index 3517a639b..eb451fa1d 100644 --- a/light/provider/http/http.go +++ b/light/provider/http/http.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/rand" + "net/url" "strings" "time" @@ -17,8 +18,8 @@ import ( ) var defaultOptions = Options{ - MaxRetryAttempts: 10, - Timeout: 5 * time.Second, + MaxRetryAttempts: 5, + Timeout: 3 * time.Second, } // http provider uses an RPC client to obtain the necessary information. @@ -146,29 +147,26 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato } } - case *rpctypes.RPCError: - // Check if we got something other than internal error. This shouldn't happen unless the RPC module - // or light client has been tampered with. If we do get this error, stop the connection with the - // peer and return an error - if e.Code != -32603 { - return nil, provider.ErrBadLightBlock{Reason: errors.New(e.Data)} + case *url.Error: + if e.Timeout() { + // request timed out: we wait and try again with exponential backoff + time.Sleep(backoffTimeout(uint16(attempt))) + continue } + return nil, provider.ErrBadLightBlock{Reason: e} + case *rpctypes.RPCError: // check if the error indicates that the peer doesn't have the block if strings.Contains(e.Data, ctypes.ErrHeightNotAvailable.Error()) || strings.Contains(e.Data, ctypes.ErrHeightExceedsChainHead.Error()) { return nil, provider.ErrLightBlockNotFound } - - // we wait and try again with exponential backoff - // TODO: If we can, we should check if the error is purely because the node failed to respond in - // time. If this is the case then we continue, else we should stop straight away - time.Sleep(backoffTimeout(uint16(attempt))) - continue + return nil, provider.ErrBadLightBlock{Reason: e} default: - // something has happened to the RPC module if we are not receiving errors of type RPCError - panic(fmt.Errorf("unexpected error type: %w", err)) + // If we don't know the error then by default we return a bad light block error and + // terminate the connection with the peer. + return nil, provider.ErrBadLightBlock{Reason: e} } // update the total and increment the page index so we can fetch the @@ -196,6 +194,14 @@ func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHe case nil: // success!! return &commit.SignedHeader, nil + case *url.Error: + if e.Timeout() { + // we wait and try again with exponential backoff + time.Sleep(backoffTimeout(uint16(attempt))) + continue + } + return nil, provider.ErrBadLightBlock{Reason: e} + case *rpctypes.RPCError: // Check if we got something other than internal error. This shouldn't happen unless the RPC module // or light client has been tampered with. If we do get this error, stop the connection with the @@ -210,15 +216,10 @@ func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHe return nil, provider.ErrLightBlockNotFound } - // we wait and try again with exponential backoff - // TODO: If we can, we should check if the error is purely because the node failed to respond in - // time. If this is the case then we continue, else we should stop straight away - time.Sleep(backoffTimeout(uint16(attempt))) - continue - default: - // something has happened to the RPC module if we are not receiving errors of type RPCError - panic(fmt.Errorf("unexpected error type: %w", err)) + // If we don't know the error then by default we return a bad light block error and + // terminate the connection with the peer. + return nil, provider.ErrBadLightBlock{Reason: e} } } return nil, provider.ErrNoResponse diff --git a/light/rpc/client.go b/light/rpc/client.go index 29f2a638a..440a85f22 100644 --- a/light/rpc/client.go +++ b/light/rpc/client.go @@ -507,7 +507,7 @@ func (c *Client) updateLightClientIfNeededTo(ctx context.Context, height *int64) l, err = c.lc.VerifyLightBlockAtHeight(ctx, *height, time.Now()) } if err != nil { - return nil, fmt.Errorf("failed to update light client to %d: %w", height, err) + return nil, fmt.Errorf("failed to update light client: %w", err) } return l, nil } diff --git a/rpc/jsonrpc/client/http_json_client.go b/rpc/jsonrpc/client/http_json_client.go index 9041b210e..479aa7266 100644 --- a/rpc/jsonrpc/client/http_json_client.go +++ b/rpc/jsonrpc/client/http_json_client.go @@ -183,11 +183,8 @@ func (c *Client) Call( } httpResponse, err := c.client.Do(httpRequest) - if e, ok := err.(*url.Error); ok && e.Timeout() { - panic("Hello world") - } if err != nil { - return nil, fmt.Errorf("request failed: %w", err) + return nil, err } defer httpResponse.Body.Close()