From 99d8c80f14d32f0d1679d594576cdf70727cb107 Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Thu, 6 Feb 2025 01:25:51 -0800 Subject: [PATCH 1/9] Update http batch request to handle singletons --- tm2/pkg/bft/rpc/lib/client/http/client.go | 32 ++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 288eca57300..035fc0cec8b 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -9,6 +9,7 @@ import ( "io" "net" "net/http" + "reflect" "strings" types "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/types" @@ -113,9 +114,11 @@ func sendRequestCommon[T requestType, R responseType]( request T, ) (R, error) { // Marshal the request + fmt.Printf("#### HOW OFTEN DOES THIS HAPPEN +++++####\n") requestBytes, err := json.Marshal(request) if err != nil { - return nil, fmt.Errorf("unable to JSON-marshal the request, %w", err) + var zero R + return zero, fmt.Errorf("unable to JSON-marshal the request, %w", err) } // Craft the request @@ -125,7 +128,8 @@ func sendRequestCommon[T requestType, R responseType]( bytes.NewBuffer(requestBytes), ) if err != nil { - return nil, fmt.Errorf("unable to create request, %w", err) + var zero R + return zero, fmt.Errorf("unable to create request, %w", err) } // Set the header content type @@ -134,25 +138,41 @@ func sendRequestCommon[T requestType, R responseType]( // Execute the request httpResponse, err := client.Do(req.WithContext(ctx)) if err != nil { - return nil, fmt.Errorf("unable to send request, %w", err) + var zero R + return zero, fmt.Errorf("unable to send request, %w", err) } defer httpResponse.Body.Close() //nolint: errcheck // Parse the response code if !isOKStatus(httpResponse.StatusCode) { - return nil, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) + var zero R + return zero, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) } // Parse the response body responseBytes, err := io.ReadAll(httpResponse.Body) if err != nil { - return nil, fmt.Errorf("unable to read response body, %w", err) + var zero R + return zero, fmt.Errorf("unable to read response body, %w", err) } var response R if err := json.Unmarshal(responseBytes, &response); err != nil { - return nil, fmt.Errorf("unable to unmarshal response body, %w", err) + // If unmarshal fails, and if R is a slice, try to unmarshal as a single element. + var zero R + rType := reflect.TypeOf(zero) + if rType.Kind() == reflect.Slice { + elemType := rType.Elem() + singlePtr := reflect.New(elemType).Interface() + if err2 := json.Unmarshal(responseBytes, singlePtr); err2 == nil { + newSlice := reflect.MakeSlice(rType, 0, 1) + newSlice = reflect.Append(newSlice, reflect.ValueOf(singlePtr).Elem()) + response = newSlice.Interface().(R) + return response, nil + } + } + return zero, fmt.Errorf("unable to unmarshal response body, %w", err) } return response, nil From 34bc8b164cd6b34051d6867fa6ff9b2c7fbc847a Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Thu, 6 Feb 2025 08:58:53 -0800 Subject: [PATCH 2/9] remove print statement for pr --- tm2/pkg/bft/rpc/lib/client/http/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 035fc0cec8b..43ff4c8ec95 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -114,7 +114,6 @@ func sendRequestCommon[T requestType, R responseType]( request T, ) (R, error) { // Marshal the request - fmt.Printf("#### HOW OFTEN DOES THIS HAPPEN +++++####\n") requestBytes, err := json.Marshal(request) if err != nil { var zero R From 2bf570719389bcc5ddf7d7bac2943e714d7a4474 Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Thu, 6 Feb 2025 22:56:26 -0800 Subject: [PATCH 3/9] client side and server side marshalling refresh --- tm2/pkg/bft/rpc/lib/client/http/client.go | 69 ++++++------ tm2/pkg/bft/rpc/lib/server/handlers.go | 123 ++++++++++++---------- 2 files changed, 100 insertions(+), 92 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 43ff4c8ec95..09cdd5373e3 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -9,7 +9,6 @@ import ( "io" "net" "net/http" - "reflect" "strings" types "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/types" @@ -112,69 +111,67 @@ func sendRequestCommon[T requestType, R responseType]( client *http.Client, rpcURL string, request T, -) (R, error) { +) (res R, err error) { // Marshal the request requestBytes, err := json.Marshal(request) if err != nil { - var zero R - return zero, fmt.Errorf("unable to JSON-marshal the request, %w", err) + return res, fmt.Errorf("unable to JSON-marshal the request, %w", err) } // Craft the request - req, err := http.NewRequest( - http.MethodPost, - rpcURL, - bytes.NewBuffer(requestBytes), - ) + req, err := http.NewRequest(http.MethodPost, rpcURL, bytes.NewBuffer(requestBytes)) if err != nil { - var zero R - return zero, fmt.Errorf("unable to create request, %w", err) + return res, fmt.Errorf("unable to create request: %w", err) } - - // Set the header content type req.Header.Set("Content-Type", "application/json") // Execute the request httpResponse, err := client.Do(req.WithContext(ctx)) if err != nil { - var zero R - return zero, fmt.Errorf("unable to send request, %w", err) + return res, fmt.Errorf("unable to send request: %w", err) } - defer httpResponse.Body.Close() //nolint: errcheck + defer httpResponse.Body.Close() // Parse the response code if !isOKStatus(httpResponse.StatusCode) { - var zero R - return zero, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) + return res, fmt.Errorf("invalid status code received: %d", httpResponse.StatusCode) } // Parse the response body responseBytes, err := io.ReadAll(httpResponse.Body) if err != nil { - var zero R - return zero, fmt.Errorf("unable to read response body, %w", err) + return res, fmt.Errorf("unable to read response body: %w", err) } - var response R - - if err := json.Unmarshal(responseBytes, &response); err != nil { - // If unmarshal fails, and if R is a slice, try to unmarshal as a single element. - var zero R - rType := reflect.TypeOf(zero) - if rType.Kind() == reflect.Slice { - elemType := rType.Elem() - singlePtr := reflect.New(elemType).Interface() - if err2 := json.Unmarshal(responseBytes, singlePtr); err2 == nil { - newSlice := reflect.MakeSlice(rType, 0, 1) - newSlice = reflect.Append(newSlice, reflect.ValueOf(singlePtr).Elem()) - response = newSlice.Interface().(R) - return response, nil + // Attempt to unmarshal into the expected response type. + var zero R + switch any(zero).(type) { + case *types.RPCResponse: + // Expected type is a pointer to a single RPCResponse. + var singleResponse types.RPCResponse + if err = json.Unmarshal(responseBytes, &singleResponse); err != nil { + return zero, fmt.Errorf("unable to unmarshal response body: %w", err) + } + res = any(&singleResponse).(R) + case types.RPCResponses: + // Expected type is a slice of RPCResponse. + var responses types.RPCResponses + // First, try unmarshalling as a slice. + if err = json.Unmarshal(responseBytes, &responses); err != nil { + // If that fails, try unmarshalling as a single element. + var response types.RPCResponse + if err2 := json.Unmarshal(responseBytes, &response); err2 == nil { + responses = types.RPCResponses{response} + } else { + return zero, fmt.Errorf("unable to unmarshal response body: %w", err) } } - return zero, fmt.Errorf("unable to unmarshal response body, %w", err) + res = any(responses).(R) + default: + return zero, fmt.Errorf("unexpected response type") } - return response, nil + return res, nil } // DefaultHTTPClient is used to create an http client with some default parameters. diff --git a/tm2/pkg/bft/rpc/lib/server/handlers.go b/tm2/pkg/bft/rpc/lib/server/handlers.go index b91db806342..52a7486c91d 100644 --- a/tm2/pkg/bft/rpc/lib/server/handlers.go +++ b/tm2/pkg/bft/rpc/lib/server/handlers.go @@ -140,74 +140,85 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger *slog.Logger) http.H return } - // first try to unmarshal the incoming request as an array of RPC requests - var ( - requests types.RPCRequests - responses types.RPCResponses - ) - - // isRPCRequestArray is used to determine if the incoming payload is an array of requests. - // This flag helps decide whether to return an array of responses (for batch requests) or a single response. - isRPCRequestArray := true + // --- Branch 1: Attempt to Unmarshal as a Batch (Slice) of Requests --- + var requests []types.RPCRequest + if err := json.Unmarshal(b, &requests); err == nil { + var responses types.RPCResponses + for _, req := range requests { + if resp := processRequest(r, req, funcMap, logger); resp != nil { + responses = append(responses, *resp) + } + } - if err := json.Unmarshal(b, &requests); err != nil { - // next, try to unmarshal as a single request - var request types.RPCRequest - if err := json.Unmarshal(b, &request); err != nil { - WriteRPCResponseHTTP(w, types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "error unmarshalling request"))) + if len(responses) > 0 { + WriteRPCResponseArrayHTTP(w, responses) return } - requests = []types.RPCRequest{request} - isRPCRequestArray = false } - for _, request := range requests { - request := request - // A Notification is a Request object without an "id" member. - // The Server MUST NOT reply to a Notification, including those that are within a batch request. - if request.ID == types.JSONRPCStringID("") { - logger.Debug("HTTPJSONRPC received a notification, skipping... (please send a non-empty ID if you want to call a method)") - continue - } - if len(r.URL.Path) > 1 { - responses = append(responses, types.RPCInvalidRequestError(request.ID, errors.New("path %s is invalid", r.URL.Path))) - continue - } - rpcFunc, ok := funcMap[request.Method] - if !ok || rpcFunc.ws { - responses = append(responses, types.RPCMethodNotFoundError(request.ID)) - continue - } - ctx := &types.Context{JSONReq: &request, HTTPReq: r} - args := []reflect.Value{reflect.ValueOf(ctx)} - if len(request.Params) > 0 { - fnArgs, err := jsonParamsToArgs(rpcFunc, request.Params) - if err != nil { - responses = append(responses, types.RPCInvalidParamsError(request.ID, errors.Wrap(err, "error converting json params to arguments"))) - continue - } - args = append(args, fnArgs...) - } - returns := rpcFunc.f.Call(args) - logger.Info("HTTPJSONRPC", "method", request.Method, "args", args, "returns", returns) - result, err := unreflectResult(returns) - if err != nil { - responses = append(responses, types.RPCInternalError(request.ID, err)) - continue + // --- Branch 2: Attempt to Unmarshal as a Single Request --- + var request types.RPCRequest + if err := json.Unmarshal(b, &request); err == nil { + if resp := processRequest(r, request, funcMap, logger); resp != nil { + WriteRPCResponseHTTP(w, *resp) + return } - responses = append(responses, types.NewRPCSuccessResponse(request.ID, result)) - } - if len(responses) == 0 { + } else { + WriteRPCResponseHTTP(w, types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "error unmarshalling request"))) return } + } +} - if isRPCRequestArray { - WriteRPCResponseArrayHTTP(w, responses) - return +// parseAndFilterRequest checks and processes a single JSON-RPC request. +// If the request should produce a response, it returns a pointer to that response. +// Otherwise (e.g. if the request is a notification or fails validation), +// it returns nil. +func processRequest(r *http.Request, req types.RPCRequest, funcMap map[string]*RPCFunc, logger *slog.Logger) *types.RPCResponse { + // Skip notifications (an empty ID indicates no response should be sent) + if req.ID == types.JSONRPCStringID("") { + logger.Debug("Skipping notification (empty ID)") + return nil + } + + // Check that the URL path is valid (assume only "/" is acceptable) + if len(r.URL.Path) > 1 { + resp := types.RPCInvalidRequestError(req.ID, fmt.Errorf("invalid path: %s", r.URL.Path)) + return &resp + } + + // Look up the requested method in the function map. + rpcFunc, ok := funcMap[req.Method] + if !ok || rpcFunc.ws { + resp := types.RPCMethodNotFoundError(req.ID) + return &resp + } + + ctx := &types.Context{JSONReq: &req, HTTPReq: r} + args := []reflect.Value{reflect.ValueOf(ctx)} + if len(req.Params) > 0 { + fnArgs, err := jsonParamsToArgs(rpcFunc, req.Params) + if err != nil { + resp := types.RPCInvalidParamsError(req.ID, errors.Wrap(err, "error converting json params to arguments")) + return &resp } + args = append(args, fnArgs...) + } - WriteRPCResponseHTTP(w, responses[0]) + // Call the RPC function using reflection. + returns := rpcFunc.f.Call(args) + logger.Info("HTTPJSONRPC", "method", req.Method, "args", args, "returns", returns) + + // Convert the reflection return values into a result value for JSON serialization. + result, err := unreflectResult(returns) + if err != nil { + resp := types.RPCInternalError(req.ID, err) + return &resp } + + // Build and return a successful response. + resp := types.NewRPCSuccessResponse(req.ID, result) + return &resp } func handleInvalidJSONRPCPaths(next http.HandlerFunc) http.HandlerFunc { From 626403619ff14a8830f5f65df731293ab6f0042d Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Fri, 7 Feb 2025 10:02:52 -0800 Subject: [PATCH 4/9] small changes to handlers.go --- tm2/pkg/bft/rpc/lib/server/handlers.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/server/handlers.go b/tm2/pkg/bft/rpc/lib/server/handlers.go index 52a7486c91d..8269bc8bf8a 100644 --- a/tm2/pkg/bft/rpc/lib/server/handlers.go +++ b/tm2/pkg/bft/rpc/lib/server/handlers.go @@ -141,7 +141,7 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger *slog.Logger) http.H } // --- Branch 1: Attempt to Unmarshal as a Batch (Slice) of Requests --- - var requests []types.RPCRequest + var requests types.RPCRequests if err := json.Unmarshal(b, &requests); err == nil { var responses types.RPCResponses for _, req := range requests { @@ -170,10 +170,9 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger *slog.Logger) http.H } } -// parseAndFilterRequest checks and processes a single JSON-RPC request. +// processRequest checks and processes a single JSON-RPC request. // If the request should produce a response, it returns a pointer to that response. -// Otherwise (e.g. if the request is a notification or fails validation), -// it returns nil. +// Otherwise (e.g. if the request is a notification or fails validation), it returns nil. func processRequest(r *http.Request, req types.RPCRequest, funcMap map[string]*RPCFunc, logger *slog.Logger) *types.RPCResponse { // Skip notifications (an empty ID indicates no response should be sent) if req.ID == types.JSONRPCStringID("") { From 239f699950c30e8c9e6a0792232cc8aea3ed7e23 Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Fri, 7 Feb 2025 10:57:33 -0800 Subject: [PATCH 5/9] updated client.go -- much simpler control flow --- tm2/pkg/bft/rpc/lib/client/http/client.go | 47 ++++++++--------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 09cdd5373e3..96dc3487bb4 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -111,67 +111,50 @@ func sendRequestCommon[T requestType, R responseType]( client *http.Client, rpcURL string, request T, -) (res R, err error) { +) (response R, err error) { // Marshal the request requestBytes, err := json.Marshal(request) if err != nil { - return res, fmt.Errorf("unable to JSON-marshal the request, %w", err) + return response, fmt.Errorf("unable to JSON-marshal the request, %w", err) } // Craft the request req, err := http.NewRequest(http.MethodPost, rpcURL, bytes.NewBuffer(requestBytes)) if err != nil { - return res, fmt.Errorf("unable to create request: %w", err) + return response, fmt.Errorf("unable to create request, %w", err) } req.Header.Set("Content-Type", "application/json") // Execute the request httpResponse, err := client.Do(req.WithContext(ctx)) if err != nil { - return res, fmt.Errorf("unable to send request: %w", err) + return response, fmt.Errorf("unable to send request, %w", err) } defer httpResponse.Body.Close() // Parse the response code if !isOKStatus(httpResponse.StatusCode) { - return res, fmt.Errorf("invalid status code received: %d", httpResponse.StatusCode) + return response, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) } // Parse the response body responseBytes, err := io.ReadAll(httpResponse.Body) if err != nil { - return res, fmt.Errorf("unable to read response body: %w", err) + return response, fmt.Errorf("unable to read response body, %w", err) } // Attempt to unmarshal into the expected response type. - var zero R - switch any(zero).(type) { - case *types.RPCResponse: - // Expected type is a pointer to a single RPCResponse. - var singleResponse types.RPCResponse - if err = json.Unmarshal(responseBytes, &singleResponse); err != nil { - return zero, fmt.Errorf("unable to unmarshal response body: %w", err) - } - res = any(&singleResponse).(R) - case types.RPCResponses: - // Expected type is a slice of RPCResponse. - var responses types.RPCResponses - // First, try unmarshalling as a slice. - if err = json.Unmarshal(responseBytes, &responses); err != nil { - // If that fails, try unmarshalling as a single element. - var response types.RPCResponse - if err2 := json.Unmarshal(responseBytes, &response); err2 == nil { - responses = types.RPCResponses{response} - } else { - return zero, fmt.Errorf("unable to unmarshal response body: %w", err) - } - } - res = any(responses).(R) - default: - return zero, fmt.Errorf("unexpected response type") + var singleResponse types.RPCResponse + var batchResponse types.RPCResponses + if err = json.Unmarshal(responseBytes, &batchResponse); err == nil { + response = any(batchResponse).(R) + } else if err = json.Unmarshal(responseBytes, &singleResponse); err == nil { + response = any(singleResponse).(R) + } else { + return response, fmt.Errorf("unable to unmarshal response body: %w", err) } - return res, nil + return response, nil } // DefaultHTTPClient is used to create an http client with some default parameters. From 4c2055ea704983b1eefc7741afa1ebcf4e58ddde Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Sat, 8 Feb 2025 17:34:10 -0800 Subject: [PATCH 6/9] robust client side response unmarshalling --- tm2/pkg/bft/rpc/lib/client/http/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 96dc3487bb4..1bbcd736c78 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -144,7 +144,7 @@ func sendRequestCommon[T requestType, R responseType]( } // Attempt to unmarshal into the expected response type. - var singleResponse types.RPCResponse + var singleResponse *types.RPCResponse var batchResponse types.RPCResponses if err = json.Unmarshal(responseBytes, &batchResponse); err == nil { response = any(batchResponse).(R) From df10bbf0d1dd610dd348f3ff1f54a86a442c8c74 Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Sat, 8 Feb 2025 17:51:58 -0800 Subject: [PATCH 7/9] robust client side response unmarshalling --- tm2/pkg/bft/rpc/lib/client/http/client.go | 39 +++---- tm2/pkg/bft/rpc/lib/server/handlers.go | 122 ++++++++++------------ 2 files changed, 76 insertions(+), 85 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 1bbcd736c78..288eca57300 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -111,47 +111,48 @@ func sendRequestCommon[T requestType, R responseType]( client *http.Client, rpcURL string, request T, -) (response R, err error) { +) (R, error) { // Marshal the request requestBytes, err := json.Marshal(request) if err != nil { - return response, fmt.Errorf("unable to JSON-marshal the request, %w", err) + return nil, fmt.Errorf("unable to JSON-marshal the request, %w", err) } // Craft the request - req, err := http.NewRequest(http.MethodPost, rpcURL, bytes.NewBuffer(requestBytes)) + req, err := http.NewRequest( + http.MethodPost, + rpcURL, + bytes.NewBuffer(requestBytes), + ) if err != nil { - return response, fmt.Errorf("unable to create request, %w", err) + return nil, fmt.Errorf("unable to create request, %w", err) } + + // Set the header content type req.Header.Set("Content-Type", "application/json") // Execute the request httpResponse, err := client.Do(req.WithContext(ctx)) if err != nil { - return response, fmt.Errorf("unable to send request, %w", err) + return nil, fmt.Errorf("unable to send request, %w", err) } - defer httpResponse.Body.Close() + defer httpResponse.Body.Close() //nolint: errcheck // Parse the response code if !isOKStatus(httpResponse.StatusCode) { - return response, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) + return nil, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) } // Parse the response body responseBytes, err := io.ReadAll(httpResponse.Body) if err != nil { - return response, fmt.Errorf("unable to read response body, %w", err) - } - - // Attempt to unmarshal into the expected response type. - var singleResponse *types.RPCResponse - var batchResponse types.RPCResponses - if err = json.Unmarshal(responseBytes, &batchResponse); err == nil { - response = any(batchResponse).(R) - } else if err = json.Unmarshal(responseBytes, &singleResponse); err == nil { - response = any(singleResponse).(R) - } else { - return response, fmt.Errorf("unable to unmarshal response body: %w", err) + return nil, fmt.Errorf("unable to read response body, %w", err) + } + + var response R + + if err := json.Unmarshal(responseBytes, &response); err != nil { + return nil, fmt.Errorf("unable to unmarshal response body, %w", err) } return response, nil diff --git a/tm2/pkg/bft/rpc/lib/server/handlers.go b/tm2/pkg/bft/rpc/lib/server/handlers.go index 8269bc8bf8a..b91db806342 100644 --- a/tm2/pkg/bft/rpc/lib/server/handlers.go +++ b/tm2/pkg/bft/rpc/lib/server/handlers.go @@ -140,84 +140,74 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger *slog.Logger) http.H return } - // --- Branch 1: Attempt to Unmarshal as a Batch (Slice) of Requests --- - var requests types.RPCRequests - if err := json.Unmarshal(b, &requests); err == nil { - var responses types.RPCResponses - for _, req := range requests { - if resp := processRequest(r, req, funcMap, logger); resp != nil { - responses = append(responses, *resp) - } - } + // first try to unmarshal the incoming request as an array of RPC requests + var ( + requests types.RPCRequests + responses types.RPCResponses + ) + + // isRPCRequestArray is used to determine if the incoming payload is an array of requests. + // This flag helps decide whether to return an array of responses (for batch requests) or a single response. + isRPCRequestArray := true - if len(responses) > 0 { - WriteRPCResponseArrayHTTP(w, responses) + if err := json.Unmarshal(b, &requests); err != nil { + // next, try to unmarshal as a single request + var request types.RPCRequest + if err := json.Unmarshal(b, &request); err != nil { + WriteRPCResponseHTTP(w, types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "error unmarshalling request"))) return } + requests = []types.RPCRequest{request} + isRPCRequestArray = false } - // --- Branch 2: Attempt to Unmarshal as a Single Request --- - var request types.RPCRequest - if err := json.Unmarshal(b, &request); err == nil { - if resp := processRequest(r, request, funcMap, logger); resp != nil { - WriteRPCResponseHTTP(w, *resp) - return + for _, request := range requests { + request := request + // A Notification is a Request object without an "id" member. + // The Server MUST NOT reply to a Notification, including those that are within a batch request. + if request.ID == types.JSONRPCStringID("") { + logger.Debug("HTTPJSONRPC received a notification, skipping... (please send a non-empty ID if you want to call a method)") + continue } - } else { - WriteRPCResponseHTTP(w, types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "error unmarshalling request"))) + if len(r.URL.Path) > 1 { + responses = append(responses, types.RPCInvalidRequestError(request.ID, errors.New("path %s is invalid", r.URL.Path))) + continue + } + rpcFunc, ok := funcMap[request.Method] + if !ok || rpcFunc.ws { + responses = append(responses, types.RPCMethodNotFoundError(request.ID)) + continue + } + ctx := &types.Context{JSONReq: &request, HTTPReq: r} + args := []reflect.Value{reflect.ValueOf(ctx)} + if len(request.Params) > 0 { + fnArgs, err := jsonParamsToArgs(rpcFunc, request.Params) + if err != nil { + responses = append(responses, types.RPCInvalidParamsError(request.ID, errors.Wrap(err, "error converting json params to arguments"))) + continue + } + args = append(args, fnArgs...) + } + returns := rpcFunc.f.Call(args) + logger.Info("HTTPJSONRPC", "method", request.Method, "args", args, "returns", returns) + result, err := unreflectResult(returns) + if err != nil { + responses = append(responses, types.RPCInternalError(request.ID, err)) + continue + } + responses = append(responses, types.NewRPCSuccessResponse(request.ID, result)) + } + if len(responses) == 0 { return } - } -} - -// processRequest checks and processes a single JSON-RPC request. -// If the request should produce a response, it returns a pointer to that response. -// Otherwise (e.g. if the request is a notification or fails validation), it returns nil. -func processRequest(r *http.Request, req types.RPCRequest, funcMap map[string]*RPCFunc, logger *slog.Logger) *types.RPCResponse { - // Skip notifications (an empty ID indicates no response should be sent) - if req.ID == types.JSONRPCStringID("") { - logger.Debug("Skipping notification (empty ID)") - return nil - } - - // Check that the URL path is valid (assume only "/" is acceptable) - if len(r.URL.Path) > 1 { - resp := types.RPCInvalidRequestError(req.ID, fmt.Errorf("invalid path: %s", r.URL.Path)) - return &resp - } - - // Look up the requested method in the function map. - rpcFunc, ok := funcMap[req.Method] - if !ok || rpcFunc.ws { - resp := types.RPCMethodNotFoundError(req.ID) - return &resp - } - ctx := &types.Context{JSONReq: &req, HTTPReq: r} - args := []reflect.Value{reflect.ValueOf(ctx)} - if len(req.Params) > 0 { - fnArgs, err := jsonParamsToArgs(rpcFunc, req.Params) - if err != nil { - resp := types.RPCInvalidParamsError(req.ID, errors.Wrap(err, "error converting json params to arguments")) - return &resp + if isRPCRequestArray { + WriteRPCResponseArrayHTTP(w, responses) + return } - args = append(args, fnArgs...) - } - // Call the RPC function using reflection. - returns := rpcFunc.f.Call(args) - logger.Info("HTTPJSONRPC", "method", req.Method, "args", args, "returns", returns) - - // Convert the reflection return values into a result value for JSON serialization. - result, err := unreflectResult(returns) - if err != nil { - resp := types.RPCInternalError(req.ID, err) - return &resp + WriteRPCResponseHTTP(w, responses[0]) } - - // Build and return a successful response. - resp := types.NewRPCSuccessResponse(req.ID, result) - return &resp } func handleInvalidJSONRPCPaths(next http.HandlerFunc) http.HandlerFunc { From 9eb47131a008980233184b224895d8b282e5c007 Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Sat, 8 Feb 2025 23:42:43 -0800 Subject: [PATCH 8/9] robust client unmarshalling for http batch requests --- tm2/pkg/bft/rpc/lib/client/http/client.go | 39 +++++++------- .../bft/rpc/lib/client/http/client_test.go | 52 +++++++++++++++++++ 2 files changed, 71 insertions(+), 20 deletions(-) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client.go b/tm2/pkg/bft/rpc/lib/client/http/client.go index 288eca57300..1bbcd736c78 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client.go @@ -111,48 +111,47 @@ func sendRequestCommon[T requestType, R responseType]( client *http.Client, rpcURL string, request T, -) (R, error) { +) (response R, err error) { // Marshal the request requestBytes, err := json.Marshal(request) if err != nil { - return nil, fmt.Errorf("unable to JSON-marshal the request, %w", err) + return response, fmt.Errorf("unable to JSON-marshal the request, %w", err) } // Craft the request - req, err := http.NewRequest( - http.MethodPost, - rpcURL, - bytes.NewBuffer(requestBytes), - ) + req, err := http.NewRequest(http.MethodPost, rpcURL, bytes.NewBuffer(requestBytes)) if err != nil { - return nil, fmt.Errorf("unable to create request, %w", err) + return response, fmt.Errorf("unable to create request, %w", err) } - - // Set the header content type req.Header.Set("Content-Type", "application/json") // Execute the request httpResponse, err := client.Do(req.WithContext(ctx)) if err != nil { - return nil, fmt.Errorf("unable to send request, %w", err) + return response, fmt.Errorf("unable to send request, %w", err) } - defer httpResponse.Body.Close() //nolint: errcheck + defer httpResponse.Body.Close() // Parse the response code if !isOKStatus(httpResponse.StatusCode) { - return nil, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) + return response, fmt.Errorf("invalid status code received, %d", httpResponse.StatusCode) } // Parse the response body responseBytes, err := io.ReadAll(httpResponse.Body) if err != nil { - return nil, fmt.Errorf("unable to read response body, %w", err) - } - - var response R - - if err := json.Unmarshal(responseBytes, &response); err != nil { - return nil, fmt.Errorf("unable to unmarshal response body, %w", err) + return response, fmt.Errorf("unable to read response body, %w", err) + } + + // Attempt to unmarshal into the expected response type. + var singleResponse *types.RPCResponse + var batchResponse types.RPCResponses + if err = json.Unmarshal(responseBytes, &batchResponse); err == nil { + response = any(batchResponse).(R) + } else if err = json.Unmarshal(responseBytes, &singleResponse); err == nil { + response = any(singleResponse).(R) + } else { + return response, fmt.Errorf("unable to unmarshal response body: %w", err) } return response, nil diff --git a/tm2/pkg/bft/rpc/lib/client/http/client_test.go b/tm2/pkg/bft/rpc/lib/client/http/client_test.go index 0d88ee32650..34553da3ba0 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client_test.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client_test.go @@ -105,6 +105,58 @@ func createTestServer( return s } +// This test covers bug https://github.com/gnolang/gno/issues/3676 +// TestSendRequestCommon_BatchSingleResponse simulates a batch call where the batch +// contains a single request. This test stays on the client side. +func TestSendRequestCommon_BatchSingleResponse(t *testing.T) { + t.Parallel() + + // Step 1: Create a dummy batch request with a single item. + singleRequest := types.RPCRequest{ + JSONRPC: "2.0", + ID: types.JSONRPCStringID("1"), + } + batchRequest := types.RPCRequests{singleRequest} // types.RPCRequests is defined as []RPCRequest + + // Step 2: Create the expected batch response. + expectedResp := types.RPCResponse{ + JSONRPC: "2.0", + ID: singleRequest.ID, + } + expectedBatchResponse := types.RPCResponses{expectedResp} // types.RPCResponses is defined as []*RPCResponse + + // Step 3: Marshal the expected batch response as a JSON array. + respBytes, err := json.Marshal(expectedBatchResponse) + require.NoError(t, err) + + // Step 4: Create a test HTTP server that always returns the JSON array. + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write(respBytes) + require.NoError(t, err) + }) + ts := httptest.NewServer(handler) + defer ts.Close() + + // Step 5: Create an HTTP client and a context with timeout. + httpClient := ts.Client() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Step 6: Call sendRequestCommon with the batch request. + // We choose R to be types.RPCResponses so we expect a batch (slice) in return. + actualBatchResponse, err := sendRequestCommon[types.RPCRequests, types.RPCResponses](ctx, httpClient, ts.URL, batchRequest) + require.NoError(t, err) + + // Step 7: Verify that the returned value is a slice with one element. + assert.Len(t, actualBatchResponse, 1, "expected the batch response slice to have length one") + + // Step 8: Verify that the returned response fields match the expected values. + assert.Equal(t, expectedBatchResponse[0].ID, actualBatchResponse[0].ID) + assert.Equal(t, expectedBatchResponse[0].JSONRPC, actualBatchResponse[0].JSONRPC) +} + func TestClient_SendRequest(t *testing.T) { t.Parallel() From 697705e7db42083755a0ebb7ab98e2d9a19385de Mon Sep 17 00:00:00 2001 From: masonmcbride Date: Sun, 9 Feb 2025 16:30:56 -0800 Subject: [PATCH 9/9] robust client side request handling + tests --- .../bft/rpc/lib/client/http/client_test.go | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tm2/pkg/bft/rpc/lib/client/http/client_test.go b/tm2/pkg/bft/rpc/lib/client/http/client_test.go index 34553da3ba0..95909d88d2f 100644 --- a/tm2/pkg/bft/rpc/lib/client/http/client_test.go +++ b/tm2/pkg/bft/rpc/lib/client/http/client_test.go @@ -157,6 +157,76 @@ func TestSendRequestCommon_BatchSingleResponse(t *testing.T) { assert.Equal(t, expectedBatchResponse[0].JSONRPC, actualBatchResponse[0].JSONRPC) } +// TestSendRequestCommon_ErrorPaths tests error paths in sendRequestCommon without using a custom RoundTripper. +func TestSendRequestCommon_ErrorPaths(t *testing.T) { + t.Parallel() + + // 1. HTTP request creation failure: Using an invalid URL. + t.Run("HTTP request creation failure", func(t *testing.T) { + t.Parallel() + req := types.RPCRequest{JSONRPC: "2.0", ID: types.JSONRPCStringID("1")} + ctx := context.Background() + // Passing an invalid URL causes http.NewRequest to fail. + resp, err := sendRequestCommon[types.RPCRequest, *types.RPCResponse](ctx, http.DefaultClient, "://invalid-url", req) + require.Error(t, err) + assert.Nil(t, resp) + assert.Contains(t, err.Error(), "unable to create request") + }) + + // 2. Client.Do failure: Cancel the context before the request is made. + t.Run("Client.Do failure", func(t *testing.T) { + t.Parallel() + req := types.RPCRequest{JSONRPC: "2.0", ID: types.JSONRPCStringID("1")} + // Create a context that is already canceled. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + resp, err := sendRequestCommon[types.RPCRequest, *types.RPCResponse](ctx, http.DefaultClient, "http://example.com", req) + require.Error(t, err) + assert.Nil(t, resp) + // Expect the error to mention the failure to send the request. + assert.Contains(t, err.Error(), "unable to send request") + }) + + // 3. Non-OK HTTP status: Simulate a server returning a bad status code. + t.Run("Non-OK HTTP status", func(t *testing.T) { + t.Parallel() + req := types.RPCRequest{JSONRPC: "2.0", ID: types.JSONRPCStringID("1")} + // Create an httptest server that always returns 500. + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + ts := httptest.NewServer(handler) + defer ts.Close() + + ctx := context.Background() + resp, err := sendRequestCommon[types.RPCRequest, *types.RPCResponse](ctx, ts.Client(), ts.URL, req) + require.Error(t, err) + assert.Nil(t, resp) + assert.Contains(t, err.Error(), "invalid status code") + }) + + // 4. JSON unmarshal failure: Simulate invalid JSON in the response. + t.Run("JSON unmarshal failure", func(t *testing.T) { + t.Parallel() + req := types.RPCRequest{JSONRPC: "2.0", ID: types.JSONRPCStringID("1")} + // Create an httptest server that returns invalid JSON. + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte("invalid json")) + require.NoError(t, err) + }) + ts := httptest.NewServer(handler) + defer ts.Close() + + ctx := context.Background() + resp, err := sendRequestCommon[types.RPCRequest, *types.RPCResponse](ctx, ts.Client(), ts.URL, req) + require.Error(t, err) + assert.Nil(t, resp) + assert.Contains(t, err.Error(), "unable to unmarshal") + }) +} + func TestClient_SendRequest(t *testing.T) { t.Parallel()