From aab375ed1725df3cba0f89a8b7861c9d2b786935 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 19 Mar 2024 19:47:55 +0000 Subject: [PATCH] opt into optimistic retry for non200 error and any -32000--32768 error --- common/geth/handle_error.go | 73 +++++++++++++------------- common/geth/multihoming_client_test.go | 26 ++++----- 2 files changed, 50 insertions(+), 49 deletions(-) diff --git a/common/geth/handle_error.go b/common/geth/handle_error.go index 762eb5a265..7ba62f3c01 100644 --- a/common/geth/handle_error.go +++ b/common/geth/handle_error.go @@ -6,70 +6,69 @@ import ( "github.com/ethereum/go-ethereum/rpc" ) -// handleHttpError returns a boolean indicating if error atrributes to remote RPC +// handleHttpError returns a boolean indicating if the current RPC should be rotated func (f *RPCStatistics) handleHttpError(httpRespError rpc.HTTPError) bool { sc := httpRespError.StatusCode - f.Logger.Info("[HTTP Response Error]", "Status Code", sc) if sc >= 200 && sc < 300 { // 2xx error return false } - - if sc >= 400 && sc < 500 { - // 4xx error, Client Error. Alchemy documents 400,401,403,429 - // https://docs.alchemy.com/reference/error-reference - return false - } - - if sc >= 500 { - // 5xx codes, Server Error, Alchemy documents 500, 503 - return true - } - - // by default, attribute to rpc + // Default to rotation the current leader, because it allows a higher chance to get the query completed. + // When a http query failed for non2xx, either sender or remote server is at fault or both. + // Since the software cannot be patched at runtime, there is no immediate remedy when sender is at fault. + // If the fault is at receiver for any reason, defaulting to retry increase the chance of + // having a successful return. + f.Logger.Info("[HTTP Response Error]", "Status Code", sc, "Error", httpRespError) return true } -// handleJsonRPCError returns a boolean indicating if error atrributes to remote Server +// handleJsonRPCError returns a boolean indicating if the current RPC should be rotated +// It could be a http2xx error, websocket error or ipc error func (f *RPCStatistics) handleJsonRPCError(rpcError rpc.Error) bool { ec := rpcError.ErrorCode() + f.Logger.Info("[JSON RPC Response Error]", "Error Code", ec, "Error", rpcError) - // Based on JSON-RPC 2.0, https://www.jsonrpc.org/specification#error_object - // Parse Error, Invalid Request,Method not found,Invalid params,Internal error - if ec == -32700 || ec == -32600 || ec == -32601 || ec == -32602 || ec == -32603 { - return false - } - - // server error - if ec >= -32099 && ec <= -32000 { + // Based on JSON-RPC 2.0, https://www.jsonrpc.org/specification#error_object. + // Rotating in those case, because the software wants to operate in optimistic case. + // The same reason as above + if ec >= -32768 && ec <= -32000 { return true } - // execution revert, see https://docs.alchemy.com/reference/error-reference - if ec == 3 { - return false - } - - return true + // default to false, because http 2xx error can return any error code, https://docs.alchemy.com/reference/error-reference + // + // Todo, it is useful to have local information to distinguish if the current connection is a http connection. + // It is useful + return false } -// handleError returns a boolean indicating if error atrributes to remote Server +// handleError returns a boolean indicating if the current connection should be rotated. +// Because library of the sender uses geth, which supports only 3 types of connections, +// we can categorize the error as HTTP error, Websocket error and IPC error. +// +// If the error is http, non2xx error would generate HTTP error, https://github.com/ethereum/go-ethereum/blob/master/rpc/http.go#L233 +// but a 2xx http response could contain JSON RPC error, https://github.com/ethereum/go-ethereum/blob/master/rpc/http.go#L181 +// If the error is Websocket or IPC, we only look for JSON error, https://github.com/ethereum/go-ethereum/blob/master/rpc/json.go#L67 + func (f *RPCStatistics) handleError(err error) bool { var httpRespError rpc.HTTPError if errors.As(err, &httpRespError) { - // if error is http error + // if error is http error, i.e. non 2xx error, it is handled here + // if it is 2xx error, the error message is nil, https://github.com/ethereum/go-ethereum/blob/master/rpc/http.go, + // execution does not entere here. return f.handleHttpError(httpRespError) } else { - // it might be websocket error or ipc error. Parse json error code - var nonHttpRespError rpc.Error - if errors.As(err, &nonHttpRespError) { - return f.handleJsonRPCError(nonHttpRespError) + // it might be http2xx error, websocket error or ipc error. Parse json error code + var rpcError rpc.Error + if errors.As(err, &rpcError) { + return f.handleJsonRPCError(rpcError) } - // If no http response is returned, it is a connection issue, + // If no http response or no rpc response is returned, it is a connection issue, // since we can't accurately attribute the network issue to neither sender nor receiver // side. Optimistically, switch rpc client + f.Logger.Info("[Default Response Error]", err) return true } diff --git a/common/geth/multihoming_client_test.go b/common/geth/multihoming_client_test.go index fb9126a38d..c3a2ffbd41 100644 --- a/common/geth/multihoming_client_test.go +++ b/common/geth/multihoming_client_test.go @@ -53,22 +53,24 @@ func makeFailureCall(t *testing.T, client *geth.MultiHomingClient, numCall int) } } -func make500Error() rpc.HTTPError { - var httpRespError rpc.HTTPError - httpRespError.StatusCode = 500 - httpRespError.Status = "INTERNAL_ERROR" - httpRespError.Body = []byte{} - return httpRespError +func make500Error() error { + return rpc.HTTPError{ + StatusCode: 500, + Status: "INTERNAL_ERROR", + Body: []byte{}, + } } func TestMultihomingClientSenderFaultZeroRetry(t *testing.T) { - // 2xx and 4xx attributes to sender's fault, RPC should not rotate - statusCodes := []int{201, 202, 400, 401, 403, 429} + // 4xx attributes to sender's fault, RPC should not rotate + statusCodes := []int{400, 401} for _, sc := range statusCodes { - var httpRespError rpc.HTTPError - httpRespError.StatusCode = sc - httpRespError.Status = "INTERNAL_ERROR" - httpRespError.Body = []byte{} + + httpRespError := rpc.HTTPError{ + StatusCode: sc, + Status: "INTERNAL_ERROR", + Body: []byte{}, + } client, _ := makeTestMultihomingClient(0, httpRespError)