rpc: add support for batched requests/responses (#3534)

Continues from #3280 in building support for batched requests/responses in the JSON RPC (as per issue #3213).

* Add JSON RPC batching for client and server

As per #3213, this adds support for [JSON RPC batch requests and
responses](https://www.jsonrpc.org/specification#batch).

* Add additional checks to ensure client responses are the same as results

* Fix case where a notification is sent and no response is expected

* Add test to check that JSON RPC notifications in a batch are left out in responses

* Update CHANGELOG_PENDING.md

* Update PR number now that PR has been created

* Make errors start with lowercase letter

* Refactor batch functionality to be standalone

This refactors the batching functionality to rather act in a standalone
way. In light of supporting concurrent goroutines making use of the same
client, it would make sense to have batching functionality where one
could create a batch of requests per goroutine and send that batch
without interfering with a batch from another goroutine.

* Add examples for simple and batch HTTP client usage

* Check errors from writer and remove nolinter directives

* Make error strings start with lowercase letter

* Refactor examples to make them testable

* Use safer deferred shutdown for example Tendermint test node

* Recompose rpcClient interface from pre-existing interface components

* Rename WaitGroup for brevity

* Replace empty ID string with request ID

* Remove extraneous test case

* Convert first letter of errors.Wrap() messages to lowercase

* Remove extraneous function parameter

* Make variable declaration terse

* Reorder WaitGroup.Done call to help prevent race conditions in the face of failure

* Swap mutex to value representation and remove initialization

* Restore empty JSONRPC string ID in response to prevent nil

* Make JSONRPCBufferedRequest private

* Revert PR hard link in CHANGELOG_PENDING

* Add client ID for JSONRPCClient

This adds code to automatically generate a randomized client ID for the
JSONRPCClient, and adds a check of the IDs in the responses (if one was
set in the requests).

* Extract response ID validation into separate function

* Remove extraneous comments

* Reorder fields to indicate clearly which are protected by the mutex

* Refactor for loop to remove indexing

* Restructure and combine loop

* Flatten conditional block for better readability

* Make multi-variable declaration slightly more readable

* Change for loop style

* Compress error check statements

* Make function description more generic to show that we support different protocols

* Preallocate memory for request and result objects
This commit is contained in:
Thane Thomson
2019-04-17 11:10:12 -04:00
committed by Anton Kaliaev
parent 621c0e629d
commit 90465f727f
11 changed files with 747 additions and 133 deletions

View File

@ -103,7 +103,7 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, cdc *amino.Codec, logger lo
return func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body)
if err != nil {
WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(types.JSONRPCStringID(""), errors.Wrap(err, "Error reading request body")))
WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(types.JSONRPCStringID(""), errors.Wrap(err, "error reading request body")))
return
}
// if its an empty request (like from a browser),
@ -113,49 +113,59 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, cdc *amino.Codec, logger lo
return
}
var request types.RPCRequest
err = json.Unmarshal(b, &request)
if err != nil {
WriteRPCResponseHTTP(w, types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "Error unmarshalling request")))
return
}
// 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)")
return
}
if len(r.URL.Path) > 1 {
WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(request.ID, errors.Errorf("Path %s is invalid", r.URL.Path)))
return
}
rpcFunc := funcMap[request.Method]
if rpcFunc == nil || rpcFunc.ws {
WriteRPCResponseHTTP(w, types.RPCMethodNotFoundError(request.ID))
return
}
ctx := &types.Context{JSONReq: &request, HTTPReq: r}
args := []reflect.Value{reflect.ValueOf(ctx)}
if len(request.Params) > 0 {
fnArgs, err := jsonParamsToArgs(rpcFunc, cdc, request.Params)
if err != nil {
WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(request.ID, errors.Wrap(err, "Error converting json params to arguments")))
// first try to unmarshal the incoming request as an array of RPC requests
var (
requests []types.RPCRequest
responses []types.RPCResponse
)
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
}
args = append(args, fnArgs...)
requests = []types.RPCRequest{request}
}
returns := rpcFunc.f.Call(args)
logger.Info("HTTPJSONRPC", "method", request.Method, "args", args, "returns", returns)
result, err := unreflectResult(returns)
if err != nil {
WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID, err))
return
for _, request := range requests {
// 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.Errorf("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, cdc, 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(cdc, request.ID, result))
}
if len(responses) > 0 {
WriteRPCResponseArrayHTTP(w, responses)
}
WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(cdc, request.ID, result))
}
}
@ -194,7 +204,7 @@ func mapParamsToArgs(rpcFunc *RPCFunc, cdc *amino.Codec, params map[string]json.
func arrayParamsToArgs(rpcFunc *RPCFunc, cdc *amino.Codec, params []json.RawMessage, argsOffset int) ([]reflect.Value, error) {
if len(rpcFunc.argNames) != len(params) {
return nil, errors.Errorf("Expected %v parameters (%v), got %v (%v)",
return nil, errors.Errorf("expected %v parameters (%v), got %v (%v)",
len(rpcFunc.argNames), rpcFunc.argNames, len(params), params)
}
@ -236,7 +246,7 @@ func jsonParamsToArgs(rpcFunc *RPCFunc, cdc *amino.Codec, raw []byte) ([]reflect
}
// Otherwise, bad format, we cannot parse
return nil, errors.Errorf("Unknown type for JSON params: %v. Expected map or array", err)
return nil, errors.Errorf("unknown type for JSON params: %v. Expected map or array", err)
}
// rpc.json
@ -261,7 +271,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, cdc *amino.Codec, logger log.Logger) func
fnArgs, err := httpParamsToArgs(rpcFunc, cdc, r)
if err != nil {
WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(types.JSONRPCStringID(""), errors.Wrap(err, "Error converting http params to arguments")))
WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(types.JSONRPCStringID(""), errors.Wrap(err, "error converting http params to arguments")))
return
}
args = append(args, fnArgs...)
@ -372,7 +382,7 @@ func _nonJSONStringToArg(cdc *amino.Codec, rt reflect.Type, arg string) (reflect
if isHexString {
if !expectingString && !expectingByteSlice {
err := errors.Errorf("Got a hex string arg, but expected '%s'",
err := errors.Errorf("got a hex string arg, but expected '%s'",
rt.Kind().String())
return reflect.ValueOf(nil), err, false
}
@ -631,7 +641,7 @@ func (wsc *wsConnection) readRoutine() {
var request types.RPCRequest
err = json.Unmarshal(in, &request)
if err != nil {
wsc.WriteRPCResponse(types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "Error unmarshaling request")))
wsc.WriteRPCResponse(types.RPCParseError(types.JSONRPCStringID(""), errors.Wrap(err, "error unmarshaling request")))
continue
}
@ -654,7 +664,7 @@ func (wsc *wsConnection) readRoutine() {
if len(request.Params) > 0 {
fnArgs, err := jsonParamsToArgs(rpcFunc, wsc.cdc, request.Params)
if err != nil {
wsc.WriteRPCResponse(types.RPCInternalError(request.ID, errors.Wrap(err, "Error converting json params to arguments")))
wsc.WriteRPCResponse(types.RPCInternalError(request.ID, errors.Wrap(err, "error converting json params to arguments")))
continue
}
args = append(args, fnArgs...)

View File

@ -154,6 +154,72 @@ func TestRPCNotification(t *testing.T) {
require.Equal(t, len(blob), 0, "a notification SHOULD NOT be responded to by the server")
}
func TestRPCNotificationInBatch(t *testing.T) {
mux := testMux()
tests := []struct {
payload string
expectCount int
}{
{
`[
{"jsonrpc": "2.0","id": ""},
{"jsonrpc": "2.0","method":"c","id":"abc","params":["a","10"]}
]`,
1,
},
{
`[
{"jsonrpc": "2.0","id": ""},
{"jsonrpc": "2.0","method":"c","id":"abc","params":["a","10"]},
{"jsonrpc": "2.0","id": ""},
{"jsonrpc": "2.0","method":"c","id":"abc","params":["a","10"]}
]`,
2,
},
}
for i, tt := range tests {
req, _ := http.NewRequest("POST", "http://localhost/", strings.NewReader(tt.payload))
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
res := rec.Result()
// Always expecting back a JSONRPCResponse
assert.True(t, statusOK(res.StatusCode), "#%d: should always return 2XX", i)
blob, err := ioutil.ReadAll(res.Body)
if err != nil {
t.Errorf("#%d: err reading body: %v", i, err)
continue
}
var responses []types.RPCResponse
// try to unmarshal an array first
err = json.Unmarshal(blob, &responses)
if err != nil {
// if we were actually expecting an array, but got an error
if tt.expectCount > 1 {
t.Errorf("#%d: expected an array, couldn't unmarshal it\nblob: %s", i, blob)
continue
} else {
// we were expecting an error here, so let's unmarshal a single response
var response types.RPCResponse
err = json.Unmarshal(blob, &response)
if err != nil {
t.Errorf("#%d: expected successful parsing of an RPCResponse\nblob: %s", i, blob)
continue
}
// have a single-element result
responses = []types.RPCResponse{response}
}
}
if tt.expectCount != len(responses) {
t.Errorf("#%d: expected %d response(s), but got %d\nblob: %s", i, tt.expectCount, len(responses), blob)
continue
}
for _, response := range responses {
assert.NotEqual(t, response, new(types.RPCResponse), "#%d: not expecting a blank RPCResponse", i)
}
}
}
func TestUnknownRPCPath(t *testing.T) {
mux := testMux()
req, _ := http.NewRequest("GET", "http://localhost/unknownrpcpath", nil)

View File

@ -76,7 +76,7 @@ func GetParamUint(r *http.Request, param string) (uint, error) {
func GetParamRegexp(r *http.Request, param string, re *regexp.Regexp) (string, error) {
s := GetParam(r, param)
if !re.MatchString(s) {
return "", errors.Errorf(param, "Did not match regular expression %v", re.String())
return "", errors.Errorf(param, "did not match regular expression %v", re.String())
}
return s, nil
}

View File

@ -98,7 +98,9 @@ func WriteRPCResponseHTTPError(
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(httpCode)
w.Write(jsonBytes) // nolint: errcheck, gas
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
}
func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) {
@ -108,12 +110,33 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) {
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
w.Write(jsonBytes) // nolint: errcheck, gas
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
}
// WriteRPCResponseArrayHTTP will do the same as WriteRPCResponseHTTP, except it
// can write arrays of responses for batched request/response interactions via
// the JSON RPC.
func WriteRPCResponseArrayHTTP(w http.ResponseWriter, res []types.RPCResponse) {
if len(res) == 1 {
WriteRPCResponseHTTP(w, res[0])
} else {
jsonBytes, err := json.MarshalIndent(res, "", " ")
if err != nil {
panic(err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
}
}
//-----------------------------------------------------------------------------
// Wraps an HTTP handler, adding error logging.
// RecoverAndLogHandler wraps an HTTP handler, adding error logging.
// If the inner function panics, the outer function recovers, logs, sends an
// HTTP 500 error response.
func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler {
@ -191,14 +214,14 @@ func Listen(addr string, config *Config) (listener net.Listener, err error) {
parts := strings.SplitN(addr, "://", 2)
if len(parts) != 2 {
return nil, errors.Errorf(
"Invalid listening address %s (use fully formed addresses, including the tcp:// or unix:// prefix)",
"invalid listening address %s (use fully formed addresses, including the tcp:// or unix:// prefix)",
addr,
)
}
proto, addr := parts[0], parts[1]
listener, err = net.Listen(proto, addr)
if err != nil {
return nil, errors.Errorf("Failed to listen on %v: %v", addr, err)
return nil, errors.Errorf("failed to listen on %v: %v", addr, err)
}
if config.MaxOpenConnections > 0 {
listener = netutil.LimitListener(listener, config.MaxOpenConnections)