From 60a1f49a5c70a5b4cfd100a8d9f9329b7e359969 Mon Sep 17 00:00:00 2001 From: Dave Bryson Date: Fri, 26 May 2017 14:46:33 +0200 Subject: [PATCH 01/10] updated json response to match spec by @davebryson --- rpc/core/events.go | 4 +-- rpc/lib/client/http_client.go | 2 +- rpc/lib/server/handlers.go | 33 ++++++++++---------- rpc/lib/server/http_server.go | 2 +- rpc/lib/types/types.go | 57 +++++++++++++++++++++++++++-------- rpc/lib/types/types_test.go | 32 ++++++++++++++++++++ 6 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 rpc/lib/types/types_test.go diff --git a/rpc/core/events.go b/rpc/core/events.go index 4671d3417..00fd9a08c 100644 --- a/rpc/core/events.go +++ b/rpc/core/events.go @@ -2,7 +2,7 @@ package core import ( ctypes "github.com/tendermint/tendermint/rpc/core/types" - "github.com/tendermint/tendermint/rpc/lib/types" + rpctypes "github.com/tendermint/tendermint/rpc/lib/types" "github.com/tendermint/tendermint/types" ) @@ -39,7 +39,7 @@ func Subscribe(wsCtx rpctypes.WSRPCContext, event string) (*ctypes.ResultSubscri // NOTE: EventSwitch callbacks must be nonblocking // NOTE: RPCResponses of subscribed events have id suffix "#event" tmResult := &ctypes.ResultEvent{event, msg} - wsCtx.TryWriteRPCResponse(rpctypes.NewRPCResponse(wsCtx.Request.ID+"#event", tmResult, "")) + wsCtx.TryWriteRPCResponse(rpctypes.NewRPCSuccessResponse(wsCtx.Request.ID+"#event", tmResult)) }) return &ctypes.ResultSubscribe{}, nil } diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index 755c3e79c..bd3846c64 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -146,7 +146,7 @@ func unmarshalResponseBytes(responseBytes []byte, result interface{}) (interface if err != nil { return nil, errors.Errorf("Error unmarshalling rpc response: %v", err) } - errorStr := response.Error + errorStr := response.Error.Message if errorStr != "" { return nil, errors.Errorf("Response error: %v", errorStr) } diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 3b81567e4..a4df4b840 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -110,35 +110,35 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han var request types.RPCRequest err := json.Unmarshal(b, &request) if err != nil { - WriteRPCResponseHTTPError(w, http.StatusBadRequest, types.NewRPCResponse("", nil, fmt.Sprintf("Error unmarshalling request: %v", err.Error()))) + WriteRPCResponseHTTP(w, types.RPCParseError("")) return } if len(r.URL.Path) > 1 { - WriteRPCResponseHTTPError(w, http.StatusNotFound, types.NewRPCResponse(request.ID, nil, fmt.Sprintf("Invalid JSONRPC endpoint %s", r.URL.Path))) + WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(request.ID)) return } rpcFunc := funcMap[request.Method] if rpcFunc == nil { - WriteRPCResponseHTTPError(w, http.StatusNotFound, types.NewRPCResponse(request.ID, nil, "RPC method unknown: "+request.Method)) + WriteRPCResponseHTTP(w, types.RPCMethodNotFoundError(request.ID)) return } if rpcFunc.ws { - WriteRPCResponseHTTPError(w, http.StatusMethodNotAllowed, types.NewRPCResponse(request.ID, nil, "RPC method is only for websockets: "+request.Method)) + WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID)) return } args, err := jsonParamsToArgsRPC(rpcFunc, request.Params) if err != nil { - WriteRPCResponseHTTPError(w, http.StatusBadRequest, types.NewRPCResponse(request.ID, nil, fmt.Sprintf("Error converting json params to arguments: %v", err.Error()))) + WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(request.ID)) return } returns := rpcFunc.f.Call(args) logger.Info("HTTPJSONRPC", "method", request.Method, "args", args, "returns", returns) result, err := unreflectResult(returns) if err != nil { - WriteRPCResponseHTTPError(w, http.StatusInternalServerError, types.NewRPCResponse(request.ID, result, err.Error())) + WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID)) return } - WriteRPCResponseHTTP(w, types.NewRPCResponse(request.ID, result, "")) + WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(request.ID, result)) } } @@ -229,7 +229,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit // Exception for websocket endpoints if rpcFunc.ws { return func(w http.ResponseWriter, r *http.Request) { - WriteRPCResponseHTTPError(w, http.StatusMethodNotAllowed, types.NewRPCResponse("", nil, "This RPC method is only for websockets")) + WriteRPCResponseHTTP(w, types.RPCInternalError("")) } } // All other endpoints @@ -237,17 +237,17 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit logger.Debug("HTTP HANDLER", "req", r) args, err := httpParamsToArgs(rpcFunc, r) if err != nil { - WriteRPCResponseHTTPError(w, http.StatusBadRequest, types.NewRPCResponse("", nil, fmt.Sprintf("Error converting http params to args: %v", err.Error()))) + WriteRPCResponseHTTP(w, types.RPCInvalidParamsError("")) return } returns := rpcFunc.f.Call(args) logger.Info("HTTPRestRPC", "method", r.URL.Path, "args", args, "returns", returns) result, err := unreflectResult(returns) if err != nil { - WriteRPCResponseHTTPError(w, http.StatusInternalServerError, types.NewRPCResponse("", nil, err.Error())) + WriteRPCResponseHTTP(w, types.RPCInternalError("")) return } - WriteRPCResponseHTTP(w, types.NewRPCResponse("", result, "")) + WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse("", result)) } } @@ -509,8 +509,7 @@ func (wsc *wsConnection) readRoutine() { var request types.RPCRequest err = json.Unmarshal(in, &request) if err != nil { - errStr := fmt.Sprintf("Error unmarshaling data: %s", err.Error()) - wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, nil, errStr)) + wsc.WriteRPCResponse(types.RPCParseError("")) continue } @@ -518,7 +517,7 @@ func (wsc *wsConnection) readRoutine() { rpcFunc := wsc.funcMap[request.Method] if rpcFunc == nil { - wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, nil, "RPC method unknown: "+request.Method)) + wsc.WriteRPCResponse(types.RPCMethodNotFoundError(request.ID)) continue } var args []reflect.Value @@ -529,7 +528,7 @@ func (wsc *wsConnection) readRoutine() { args, err = jsonParamsToArgsRPC(rpcFunc, request.Params) } if err != nil { - wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, nil, err.Error())) + wsc.WriteRPCResponse(types.RPCInternalError(request.ID)) continue } returns := rpcFunc.f.Call(args) @@ -539,10 +538,10 @@ func (wsc *wsConnection) readRoutine() { result, err := unreflectResult(returns) if err != nil { - wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, nil, err.Error())) + wsc.WriteRPCResponse(types.RPCInternalError(request.ID)) continue } else { - wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, result, "")) + wsc.WriteRPCResponse(types.NewRPCSuccessResponse(request.ID, result)) continue } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 270321b4b..2d6f5f1ba 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -99,7 +99,7 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler // For the rest, logger.Error("Panic in RPC HTTP handler", "err", e, "stack", string(debug.Stack())) rww.WriteHeader(http.StatusInternalServerError) - WriteRPCResponseHTTP(rww, types.NewRPCResponse("", nil, fmt.Sprintf("Internal Server Error: %v", e))) + WriteRPCResponseHTTP(rww, types.RPCInternalError("")) } } diff --git a/rpc/lib/types/types.go b/rpc/lib/types/types.go index f4a2cede0..528255a83 100644 --- a/rpc/lib/types/types.go +++ b/rpc/lib/types/types.go @@ -8,6 +8,11 @@ import ( events "github.com/tendermint/tmlibs/events" ) +type RpcError struct { + Code int `json:"code"` + Message string `json:"message"` +} + type RPCRequest struct { JSONRPC string `json:"jsonrpc"` ID string `json:"id"` @@ -50,28 +55,32 @@ func ArrayToRequest(id string, method string, params []interface{}) (RPCRequest, type RPCResponse struct { JSONRPC string `json:"jsonrpc"` - ID string `json:"id"` - Result *json.RawMessage `json:"result"` - Error string `json:"error"` + ID string `json:"id,omitempty"` + Result *json.RawMessage `json:"result,omitempty"` + Error *RpcError `json:"error,omitempty"` } -func NewRPCResponse(id string, res interface{}, err string) RPCResponse { +func NewRPCSuccessResponse(id string, res interface{}) RPCResponse { var raw *json.RawMessage + if res != nil { var js []byte - js, err2 := json.Marshal(res) - if err2 == nil { - rawMsg := json.RawMessage(js) - raw = &rawMsg - } else { - err = err2.Error() + js, err := json.Marshal(res) + if err != nil { + return RPCInternalError(id) } + rawMsg := json.RawMessage(js) + raw = &rawMsg } + + return RPCResponse{JSONRPC: "2.0", ID: id, Result: raw} +} + +func NewRPCErrorResponse(id string, code int, msg string) RPCResponse { return RPCResponse{ JSONRPC: "2.0", ID: id, - Result: raw, - Error: err, + Error: &RpcError{Code: code, Message: msg}, } } @@ -83,6 +92,30 @@ func (resp RPCResponse) String() string { } } +func RPCParseError(id string) RPCResponse { + return NewRPCErrorResponse(id, -32700, "Parse error. Invalid JSON") +} + +func RPCInvalidRequestError(id string) RPCResponse { + return NewRPCErrorResponse(id, -32600, "Invalid Request") +} + +func RPCMethodNotFoundError(id string) RPCResponse { + return NewRPCErrorResponse(id, -32601, "Method not found") +} + +func RPCInvalidParamsError(id string) RPCResponse { + return NewRPCErrorResponse(id, -32602, "Invalid params") +} + +func RPCInternalError(id string) RPCResponse { + return NewRPCErrorResponse(id, -32603, "Internal error") +} + +func RPCServerError(id string) RPCResponse { + return NewRPCErrorResponse(id, -32000, "Server error") +} + //---------------------------------------- // *wsConnection implements this interface. diff --git a/rpc/lib/types/types_test.go b/rpc/lib/types/types_test.go new file mode 100644 index 000000000..14688ecee --- /dev/null +++ b/rpc/lib/types/types_test.go @@ -0,0 +1,32 @@ +package rpctypes + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +type SampleResult struct { + Value string +} + +func TestResponses(t *testing.T) { + assert := assert.New(t) + + a := NewRPCSuccessResponse("1", &SampleResult{"hello"}) + b, _ := json.Marshal(a) + s := `{"jsonrpc":"2.0","id":"1","result":{"Value":"hello"}}` + assert.Equal(string(s), string(b)) + + d := RPCParseError("1") + e, _ := json.Marshal(d) + f := `{"jsonrpc":"2.0","id":"1","error":{"code":-32700,"message":"Parse error. Invalid JSON"}}` + assert.Equal(string(f), string(e)) + + g := RPCMethodNotFoundError("2") + h, _ := json.Marshal(g) + i := `{"jsonrpc":"2.0","id":"2","error":{"code":-32601,"message":"Method not found"}}` + assert.Equal(string(h), string(i)) + +} From 6c1572c9b8c701bc6fe64ea308282ef1fe962271 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 26 May 2017 16:56:44 +0200 Subject: [PATCH 02/10] fix invalid memory address or nil pointer dereference --- rpc/lib/client/http_client.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index bd3846c64..734ac5b17 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -146,9 +146,8 @@ func unmarshalResponseBytes(responseBytes []byte, result interface{}) (interface if err != nil { return nil, errors.Errorf("Error unmarshalling rpc response: %v", err) } - errorStr := response.Error.Message - if errorStr != "" { - return nil, errors.Errorf("Response error: %v", errorStr) + if response.Error != nil && response.Error.Message != "" { + return nil, errors.Errorf("Response error: %v", response.Error.Message) } // unmarshal the RawMessage into the result err = json.Unmarshal(*response.Result, result) From f74de4cb8664f915ea8216bd76ad65d7736d4614 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 26 May 2017 17:45:09 +0200 Subject: [PATCH 03/10] include optional data field in error object ``` data A Primitive or Structured value that contains additional information about the error. This may be omitted. The value of this member is defined by the Server (e.g. detailed error information, nested errors etc.). ``` --- rpc/lib/server/handlers.go | 22 ++++++++--------- rpc/lib/server/http_server.go | 3 ++- rpc/lib/types/types.go | 45 ++++++++++++++++++++--------------- rpc/lib/types/types_test.go | 6 ++--- 4 files changed, 42 insertions(+), 34 deletions(-) diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index a4df4b840..070072d8f 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -110,11 +110,11 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han var request types.RPCRequest err := json.Unmarshal(b, &request) if err != nil { - WriteRPCResponseHTTP(w, types.RPCParseError("")) + WriteRPCResponseHTTP(w, types.RPCParseError("", errors.Wrap(err, "Error unmarshalling request"))) return } if len(r.URL.Path) > 1 { - WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(request.ID)) + WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(request.ID, errors.Errorf("Path %s is invalid", r.URL.Path))) return } rpcFunc := funcMap[request.Method] @@ -123,19 +123,19 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han return } if rpcFunc.ws { - WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID)) + WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID, errors.New("Trying to use Websocket method in non-ws context"))) return } args, err := jsonParamsToArgsRPC(rpcFunc, request.Params) if err != nil { - WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(request.ID)) + WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(request.ID, errors.Wrap(err, "Error converting json params to arguments"))) return } 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)) + WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID, err)) return } WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(request.ID, result)) @@ -229,7 +229,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit // Exception for websocket endpoints if rpcFunc.ws { return func(w http.ResponseWriter, r *http.Request) { - WriteRPCResponseHTTP(w, types.RPCInternalError("")) + WriteRPCResponseHTTP(w, types.RPCInternalError("", errors.New("Trying to use Websocket method in non-ws context"))) } } // All other endpoints @@ -237,14 +237,14 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit logger.Debug("HTTP HANDLER", "req", r) args, err := httpParamsToArgs(rpcFunc, r) if err != nil { - WriteRPCResponseHTTP(w, types.RPCInvalidParamsError("")) + WriteRPCResponseHTTP(w, types.RPCInvalidParamsError("", errors.Wrap(err, "Error converting http params to arguments"))) return } returns := rpcFunc.f.Call(args) logger.Info("HTTPRestRPC", "method", r.URL.Path, "args", args, "returns", returns) result, err := unreflectResult(returns) if err != nil { - WriteRPCResponseHTTP(w, types.RPCInternalError("")) + WriteRPCResponseHTTP(w, types.RPCInternalError("", err)) return } WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse("", result)) @@ -509,7 +509,7 @@ func (wsc *wsConnection) readRoutine() { var request types.RPCRequest err = json.Unmarshal(in, &request) if err != nil { - wsc.WriteRPCResponse(types.RPCParseError("")) + wsc.WriteRPCResponse(types.RPCParseError("", errors.Wrap(err, "Error unmarshaling request"))) continue } @@ -528,7 +528,7 @@ func (wsc *wsConnection) readRoutine() { args, err = jsonParamsToArgsRPC(rpcFunc, request.Params) } if err != nil { - wsc.WriteRPCResponse(types.RPCInternalError(request.ID)) + wsc.WriteRPCResponse(types.RPCInternalError(request.ID, errors.Wrap(err, "Error converting json params to arguments"))) continue } returns := rpcFunc.f.Call(args) @@ -538,7 +538,7 @@ func (wsc *wsConnection) readRoutine() { result, err := unreflectResult(returns) if err != nil { - wsc.WriteRPCResponse(types.RPCInternalError(request.ID)) + wsc.WriteRPCResponse(types.RPCInternalError(request.ID, err)) continue } else { wsc.WriteRPCResponse(types.NewRPCSuccessResponse(request.ID, result)) diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 2d6f5f1ba..7623337db 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -12,6 +12,7 @@ import ( "time" "github.com/pkg/errors" + types "github.com/tendermint/tendermint/rpc/lib/types" "github.com/tendermint/tmlibs/log" ) @@ -99,7 +100,7 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler // For the rest, logger.Error("Panic in RPC HTTP handler", "err", e, "stack", string(debug.Stack())) rww.WriteHeader(http.StatusInternalServerError) - WriteRPCResponseHTTP(rww, types.RPCInternalError("")) + WriteRPCResponseHTTP(rww, types.RPCInternalError("", e.(error))) } } diff --git a/rpc/lib/types/types.go b/rpc/lib/types/types.go index 528255a83..b649d1b94 100644 --- a/rpc/lib/types/types.go +++ b/rpc/lib/types/types.go @@ -5,13 +5,13 @@ import ( "fmt" "strings" + "github.com/pkg/errors" + events "github.com/tendermint/tmlibs/events" ) -type RpcError struct { - Code int `json:"code"` - Message string `json:"message"` -} +//---------------------------------------- +// REQUEST type RPCRequest struct { JSONRPC string `json:"jsonrpc"` @@ -52,6 +52,13 @@ func ArrayToRequest(id string, method string, params []interface{}) (RPCRequest, } //---------------------------------------- +// RESPONSE + +type RpcError struct { + Code int `json:"code"` + Message string `json:"message"` + Data string `json:"data,omitempty"` +} type RPCResponse struct { JSONRPC string `json:"jsonrpc"` @@ -67,7 +74,7 @@ func NewRPCSuccessResponse(id string, res interface{}) RPCResponse { var js []byte js, err := json.Marshal(res) if err != nil { - return RPCInternalError(id) + return RPCInternalError(id, errors.Wrap(err, "Error marshalling response")) } rawMsg := json.RawMessage(js) raw = &rawMsg @@ -76,11 +83,11 @@ func NewRPCSuccessResponse(id string, res interface{}) RPCResponse { return RPCResponse{JSONRPC: "2.0", ID: id, Result: raw} } -func NewRPCErrorResponse(id string, code int, msg string) RPCResponse { +func NewRPCErrorResponse(id string, code int, msg string, data string) RPCResponse { return RPCResponse{ JSONRPC: "2.0", ID: id, - Error: &RpcError{Code: code, Message: msg}, + Error: &RpcError{Code: code, Message: msg, Data: data}, } } @@ -92,28 +99,28 @@ func (resp RPCResponse) String() string { } } -func RPCParseError(id string) RPCResponse { - return NewRPCErrorResponse(id, -32700, "Parse error. Invalid JSON") +func RPCParseError(id string, err error) RPCResponse { + return NewRPCErrorResponse(id, -32700, "Parse error. Invalid JSON", err.Error()) } -func RPCInvalidRequestError(id string) RPCResponse { - return NewRPCErrorResponse(id, -32600, "Invalid Request") +func RPCInvalidRequestError(id string, err error) RPCResponse { + return NewRPCErrorResponse(id, -32600, "Invalid Request", err.Error()) } func RPCMethodNotFoundError(id string) RPCResponse { - return NewRPCErrorResponse(id, -32601, "Method not found") + return NewRPCErrorResponse(id, -32601, "Method not found", "") } -func RPCInvalidParamsError(id string) RPCResponse { - return NewRPCErrorResponse(id, -32602, "Invalid params") +func RPCInvalidParamsError(id string, err error) RPCResponse { + return NewRPCErrorResponse(id, -32602, "Invalid params", err.Error()) } -func RPCInternalError(id string) RPCResponse { - return NewRPCErrorResponse(id, -32603, "Internal error") +func RPCInternalError(id string, err error) RPCResponse { + return NewRPCErrorResponse(id, -32603, "Internal error", err.Error()) } -func RPCServerError(id string) RPCResponse { - return NewRPCErrorResponse(id, -32000, "Server error") +func RPCServerError(id string, err error) RPCResponse { + return NewRPCErrorResponse(id, -32000, "Server error", err.Error()) } //---------------------------------------- @@ -133,7 +140,7 @@ type WSRPCContext struct { } //---------------------------------------- -// sockets +// SOCKETS // // Determine if its a unix or tcp socket. // If tcp, must specify the port; `0.0.0.0` will return incorrectly as "unix" since there's no port diff --git a/rpc/lib/types/types_test.go b/rpc/lib/types/types_test.go index 14688ecee..bab42124a 100644 --- a/rpc/lib/types/types_test.go +++ b/rpc/lib/types/types_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -19,14 +20,13 @@ func TestResponses(t *testing.T) { s := `{"jsonrpc":"2.0","id":"1","result":{"Value":"hello"}}` assert.Equal(string(s), string(b)) - d := RPCParseError("1") + d := RPCParseError("1", errors.New("Hello world")) e, _ := json.Marshal(d) - f := `{"jsonrpc":"2.0","id":"1","error":{"code":-32700,"message":"Parse error. Invalid JSON"}}` + f := `{"jsonrpc":"2.0","id":"1","error":{"code":-32700,"message":"Parse error. Invalid JSON","data":"Hello world"}}` assert.Equal(string(f), string(e)) g := RPCMethodNotFoundError("2") h, _ := json.Marshal(g) i := `{"jsonrpc":"2.0","id":"2","error":{"code":-32601,"message":"Method not found"}}` assert.Equal(string(h), string(i)) - } From e1fd587ddd5589678376ed242cb5867162fa2814 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 29 May 2017 14:48:18 +0300 Subject: [PATCH 04/10] we now omit error if empty --- test/app/counter_test.sh | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/test/app/counter_test.sh b/test/app/counter_test.sh index cc5c38b25..0198f85cd 100644 --- a/test/app/counter_test.sh +++ b/test/app/counter_test.sh @@ -23,39 +23,42 @@ function getCode() { else # this wont actually work if theres an error ... echo "$R" | jq .code - fi + fi } function sendTx() { TX=$1 if [[ "$GRPC_BROADCAST_TX" == "" ]]; then - RESPONSE=`curl -s localhost:46657/broadcast_tx_commit?tx=0x$TX` - ERROR=`echo $RESPONSE | jq .error` + RESPONSE=$(curl -s localhost:46657/broadcast_tx_commit?tx=0x"$TX") + IS_ERR=$(echo "$RESPONSE" | jq 'has("error")') + ERROR=$(echo "$RESPONSE" | jq '.error') ERROR=$(echo "$ERROR" | tr -d '"') # remove surrounding quotes - RESPONSE=`echo $RESPONSE | jq .result` + RESPONSE=$(echo "$RESPONSE" | jq '.result') else if [ -f grpc_client ]; then rm grpc_client fi echo "... building grpc_client" - go build -o grpc_client grpc_client.go - RESPONSE=`./grpc_client $TX` + go build -o grpc_client grpc_client.go + RESPONSE=$(./grpc_client "$TX") + IS_ERR=false ERROR="" fi echo "RESPONSE" - echo $RESPONSE + echo "$RESPONSE" - echo $RESPONSE | jq . &> /dev/null + echo "$RESPONSE" | jq . &> /dev/null IS_JSON=$? if [[ "$IS_JSON" != "0" ]]; then + IS_ERR=true ERROR="$RESPONSE" fi - APPEND_TX_RESPONSE=`echo $RESPONSE | jq .deliver_tx` - APPEND_TX_CODE=`getCode "$APPEND_TX_RESPONSE"` - CHECK_TX_RESPONSE=`echo $RESPONSE | jq .check_tx` - CHECK_TX_CODE=`getCode "$CHECK_TX_RESPONSE"` + APPEND_TX_RESPONSE=$(echo "$RESPONSE" | jq '.deliver_tx') + APPEND_TX_CODE=$(getCode "$APPEND_TX_RESPONSE") + CHECK_TX_RESPONSE=$(echo "$RESPONSE" | jq '.check_tx') + CHECK_TX_CODE=$(getCode "$CHECK_TX_RESPONSE") echo "-------" echo "TX $TX" @@ -63,7 +66,7 @@ function sendTx() { echo "ERROR $ERROR" echo "----" - if [[ "$ERROR" != "" ]]; then + if $IS_ERR; then echo "Unexpected error sending tx ($TX): $ERROR" exit 1 fi From b700ed8e3104ed9cefcadbb654901fe73f2868b4 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 18 Jul 2017 13:01:34 +0300 Subject: [PATCH 05/10] remove check for non-empty message as it should always be present --- rpc/lib/client/http_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index 734ac5b17..b64aea30a 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -146,7 +146,7 @@ func unmarshalResponseBytes(responseBytes []byte, result interface{}) (interface if err != nil { return nil, errors.Errorf("Error unmarshalling rpc response: %v", err) } - if response.Error != nil && response.Error.Message != "" { + if response.Error != nil { return nil, errors.Errorf("Response error: %v", response.Error.Message) } // unmarshal the RawMessage into the result From 2252071866fd0cb5995926c947c251a5a1f83403 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 18 Jul 2017 13:06:37 +0300 Subject: [PATCH 06/10] update changelog --- CHANGELOG.md | 55 ++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d76fabfca..71de7925c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,34 +10,35 @@ BREAKING CHANGES: FEATURES: - Peer reputation management -- Use the chain as its own CA for nodes and validators -- Tooling to run multiple blockchains/apps, possibly in a single process -- State syncing (without transaction replay) -- Improved support for querying history and state +- Use the chain as its own CA for nodes and validators +- Tooling to run multiple blockchains/apps, possibly in a single process +- State syncing (without transaction replay) +- Improved support for querying history and state - Add authentication and rate-limitting to the RPC IMPROVEMENTS: -- Improve subtleties around mempool caching and logic -- Consensus optimizations: +- Improve subtleties around mempool caching and logic +- Consensus optimizations: - cache block parts for faster agreement after round changes - propagate block parts rarest first -- Better testing of the consensus state machine (ie. use a DSL) +- Better testing of the consensus state machine (ie. use a DSL) - Auto compiled serialization/deserialization code instead of go-wire reflection -BUG FIXES: -- Graceful handling/recovery for apps that have non-determinism or fail to halt +BUG FIXES: +- Graceful handling/recovery for apps that have non-determinism or fail to halt - Graceful handling/recovery for violations of safety, or liveness ## 0.11.0 (Date) IMPROVEMENTS: - docs: Added documentation from the tools repo to Read The Docs pipeline + - rpc: updated json response to match http://www.jsonrpc.org/specification spec ## 0.10.4 (September 5, 2017) IMPROVEMENTS: -- docs: Added Slate docs to each rpc function (see rpc/core) -- docs: Ported all website docs to Read The Docs +- docs: Added Slate docs to each rpc function (see rpc/core) +- docs: Ported all website docs to Read The Docs - config: expose some p2p params to tweak performance: RecvRate, SendRate, and MaxMsgPacketPayloadSize - rpc: Upgrade the websocket client and server, including improved auto reconnect, and proper ping/pong @@ -77,7 +78,7 @@ IMPROVEMENTS: FEATURES: - Use `--trace` to get stack traces for logged errors -- types: GenesisDoc.ValidatorHash returns the hash of the genesis validator set +- types: GenesisDoc.ValidatorHash returns the hash of the genesis validator set - types: GenesisDocFromFile parses a GenesiDoc from a JSON file IMPROVEMENTS: @@ -101,7 +102,7 @@ Also includes the Grand Repo-Merge of 2017. BREAKING CHANGES: - Config and Flags: - - The `config` map is replaced with a [`Config` struct](https://github.com/tendermint/tendermint/blob/master/config/config.go#L11), + - The `config` map is replaced with a [`Config` struct](https://github.com/tendermint/tendermint/blob/master/config/config.go#L11), containing substructs: `BaseConfig`, `P2PConfig`, `MempoolConfig`, `ConsensusConfig`, `RPCConfig` - This affects the following flags: - `--seeds` is now `--p2p.seeds` @@ -114,16 +115,16 @@ containing substructs: `BaseConfig`, `P2PConfig`, `MempoolConfig`, `ConsensusCon ``` [p2p] laddr="tcp://1.2.3.4:46656" - + [consensus] timeout_propose=1000 ``` - Use viper and `DefaultConfig() / TestConfig()` functions to handle defaults, and remove `config/tendermint` and `config/tendermint_test` - - Change some function and method signatures to + - Change some function and method signatures to - Change some [function and method signatures](https://gist.github.com/ebuchman/640d5fc6c2605f73497992fe107ebe0b) accomodate new config - Logger - - Replace static `log15` logger with a simple interface, and provide a new implementation using `go-kit`. + - Replace static `log15` logger with a simple interface, and provide a new implementation using `go-kit`. See our new [logging library](https://github.com/tendermint/tmlibs/log) and [blog post](https://tendermint.com/blog/abstracting-the-logger-interface-in-go) for more details - Levels `warn` and `notice` are removed (you may need to change them in your `config.toml`!) - Change some [function and method signatures](https://gist.github.com/ebuchman/640d5fc6c2605f73497992fe107ebe0b) to accept a logger @@ -166,7 +167,7 @@ IMPROVEMENTS: - Limit `/blockchain_info` call to return a maximum of 20 blocks - Use `.Wrap()` and `.Unwrap()` instead of eg. `PubKeyS` for `go-crypto` types - RPC JSON responses use pretty printing (via `json.MarshalIndent`) -- Color code different instances of the consensus for tests +- Color code different instances of the consensus for tests - Isolate viper to `cmd/tendermint/commands` and do not read config from file for tests @@ -194,7 +195,7 @@ IMPROVEMENTS: - WAL uses #ENDHEIGHT instead of #HEIGHT (#HEIGHT will stop working in 0.10.0) - Peers included via `--seeds`, under `seeds` in the config, or in `/dial_seeds` are now persistent, and will be reconnected to if the connection breaks -BUG FIXES: +BUG FIXES: - Fix bug in fast-sync where we stop syncing after a peer is removed, even if they're re-added later - Fix handshake replay to handle validator set changes and results of DeliverTx when we crash after app.Commit but before state.Save() @@ -210,7 +211,7 @@ message RequestQuery{ bytes data = 1; string path = 2; uint64 height = 3; - bool prove = 4; + bool prove = 4; } message ResponseQuery{ @@ -234,7 +235,7 @@ type BlockMeta struct { } ``` -- `ValidatorSet.Proposer` is exposed as a field and persisted with the `State`. Use `GetProposer()` to initialize or update after validator-set changes. +- `ValidatorSet.Proposer` is exposed as a field and persisted with the `State`. Use `GetProposer()` to initialize or update after validator-set changes. - `tendermint gen_validator` command output is now pure JSON @@ -277,7 +278,7 @@ type BlockID struct { } ``` -- `Vote` data type now includes validator address and index: +- `Vote` data type now includes validator address and index: ``` type Vote struct { @@ -297,7 +298,7 @@ type Vote struct { FEATURES: -- New message type on the ConsensusReactor, `Maj23Msg`, for peers to alert others they've seen a Maj23, +- New message type on the ConsensusReactor, `Maj23Msg`, for peers to alert others they've seen a Maj23, in order to track and handle conflicting votes intelligently to prevent Byzantine faults from causing halts: ``` @@ -320,7 +321,7 @@ IMPROVEMENTS: - Less verbose logging - Better test coverage (37% -> 49%) - Canonical SignBytes for signable types -- Write-Ahead Log for Mempool and Consensus via tmlibs/autofile +- Write-Ahead Log for Mempool and Consensus via tmlibs/autofile - Better in-process testing for the consensus reactor and byzantine faults - Better crash/restart testing for individual nodes at preset failure points, and of networks at arbitrary points - Better abstraction over timeout mechanics @@ -400,7 +401,7 @@ FEATURES: - TMSP and RPC support TCP and UNIX sockets - Addition config options including block size and consensus parameters - New WAL mode `cswal_light`; logs only the validator's own votes -- New RPC endpoints: +- New RPC endpoints: - for starting/stopping profilers, and for updating config - `/broadcast_tx_commit`, returns when tx is included in a block, else an error - `/unsafe_flush_mempool`, empties the mempool @@ -421,14 +422,14 @@ BUG FIXES: Strict versioning only began with the release of v0.7.0, in late summer 2016. The project itself began in early summer 2014 and was workable decentralized cryptocurrency software by the end of that year. -Through the course of 2015, in collaboration with Eris Industries (now Monax Indsutries), +Through the course of 2015, in collaboration with Eris Industries (now Monax Indsutries), many additional features were integrated, including an implementation from scratch of the Ethereum Virtual Machine. That implementation now forms the heart of [Burrow](https://github.com/hyperledger/burrow). In the later half of 2015, the consensus algorithm was upgraded with a more asynchronous design and a more deterministic and robust implementation. -By late 2015, frustration with the difficulty of forking a large monolithic stack to create alternative cryptocurrency designs led to the +By late 2015, frustration with the difficulty of forking a large monolithic stack to create alternative cryptocurrency designs led to the invention of the Application Blockchain Interface (ABCI), then called the Tendermint Socket Protocol (TMSP). The Ethereum Virtual Machine and various other transaction features were removed, and Tendermint was whittled down to a core consensus engine -driving an application running in another process. +driving an application running in another process. The ABCI interface and implementation were iterated on and improved over the course of 2016, until versioned history kicked in with v0.7.0. From e36c79f7137769af5ce500ab164d94ddc347d838 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 18 Jul 2017 13:08:51 +0300 Subject: [PATCH 07/10] capitalize RpcError --- rpc/lib/client/ws_client.go | 4 ++-- rpc/lib/types/types.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index f56c45e99..7bf3fc2a9 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -422,8 +422,8 @@ func (c *WSClient) readRoutine() { c.ErrorsCh <- err continue } - if response.Error != "" { - c.ErrorsCh <- errors.Errorf(response.Error) + if response.Error != nil { + c.ErrorsCh <- errors.New(response.Error.Message) continue } c.Logger.Info("got response", "resp", response.Result) diff --git a/rpc/lib/types/types.go b/rpc/lib/types/types.go index b649d1b94..8d71d6f21 100644 --- a/rpc/lib/types/types.go +++ b/rpc/lib/types/types.go @@ -54,7 +54,7 @@ func ArrayToRequest(id string, method string, params []interface{}) (RPCRequest, //---------------------------------------- // RESPONSE -type RpcError struct { +type RPCError struct { Code int `json:"code"` Message string `json:"message"` Data string `json:"data,omitempty"` @@ -64,7 +64,7 @@ type RPCResponse struct { JSONRPC string `json:"jsonrpc"` ID string `json:"id,omitempty"` Result *json.RawMessage `json:"result,omitempty"` - Error *RpcError `json:"error,omitempty"` + Error *RPCError `json:"error,omitempty"` } func NewRPCSuccessResponse(id string, res interface{}) RPCResponse { @@ -87,12 +87,12 @@ func NewRPCErrorResponse(id string, code int, msg string, data string) RPCRespon return RPCResponse{ JSONRPC: "2.0", ID: id, - Error: &RpcError{Code: code, Message: msg, Data: data}, + Error: &RPCError{Code: code, Message: msg, Data: data}, } } func (resp RPCResponse) String() string { - if resp.Error == "" { + if resp.Error == nil { return fmt.Sprintf("[%s %v]", resp.ID, resp.Result) } else { return fmt.Sprintf("[%s %s]", resp.ID, resp.Error) From 7fadde0b373a82b7698f455fb8843c00b31295d7 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 18 Sep 2017 12:02:15 -0700 Subject: [PATCH 08/10] check for request ID after receiving it --- rpc/lib/client/http_client.go | 2 +- rpc/lib/client/ws_client.go | 4 ++-- rpc/lib/server/handlers.go | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index b64aea30a..1fbaedfae 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -75,7 +75,7 @@ func NewJSONRPCClient(remote string) *JSONRPCClient { } func (c *JSONRPCClient) Call(method string, params map[string]interface{}, result interface{}) (interface{}, error) { - request, err := types.MapToRequest("", method, params) + request, err := types.MapToRequest("jsonrpc-client", method, params) if err != nil { return nil, err } diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index 7bf3fc2a9..1407073ae 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -195,7 +195,7 @@ func (c *WSClient) Send(ctx context.Context, request types.RPCRequest) error { // Call the given method. See Send description. func (c *WSClient) Call(ctx context.Context, method string, params map[string]interface{}) error { - request, err := types.MapToRequest("", method, params) + request, err := types.MapToRequest("ws-client", method, params) if err != nil { return err } @@ -205,7 +205,7 @@ func (c *WSClient) Call(ctx context.Context, method string, params map[string]in // CallWithArrayParams the given method with params in a form of array. See // Send description. func (c *WSClient) CallWithArrayParams(ctx context.Context, method string, params []interface{}) error { - request, err := types.ArrayToRequest("", method, params) + request, err := types.ArrayToRequest("ws-client", method, params) if err != nil { return err } diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 070072d8f..5cb3baddd 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -113,6 +113,11 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han WriteRPCResponseHTTP(w, types.RPCParseError("", 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 == "" { + return + } if len(r.URL.Path) > 1 { WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(request.ID, errors.Errorf("Path %s is invalid", r.URL.Path))) return @@ -513,6 +518,12 @@ func (wsc *wsConnection) readRoutine() { continue } + // 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 == "" { + continue + } + // Now, fetch the RPCFunc and execute it. rpcFunc := wsc.funcMap[request.Method] From 95875c55fc7cf94a5e1213f1ed720f3b2ca4f3b5 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 18 Sep 2017 12:04:05 -0700 Subject: [PATCH 09/10] ID must be present in both request and response from the spec: This member is REQUIRED. It MUST be the same as the value of the id member in the Request Object. If there was an error in detecting the id in the Request object (e.g. Parse error/Invalid Request), it MUST be Null. --- rpc/lib/types/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/lib/types/types.go b/rpc/lib/types/types.go index 8d71d6f21..0267c529c 100644 --- a/rpc/lib/types/types.go +++ b/rpc/lib/types/types.go @@ -62,7 +62,7 @@ type RPCError struct { type RPCResponse struct { JSONRPC string `json:"jsonrpc"` - ID string `json:"id,omitempty"` + ID string `json:"id"` Result *json.RawMessage `json:"result,omitempty"` Error *RPCError `json:"error,omitempty"` } From f8b152972f32b7833e4ef6b2479aa57db8ccb938 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 18 Sep 2017 12:26:26 -0700 Subject: [PATCH 10/10] return method not found error if somebody tries to access WS method in non-ws context --- rpc/lib/server/handlers.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 5cb3baddd..0fafd2a84 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -123,14 +123,10 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han return } rpcFunc := funcMap[request.Method] - if rpcFunc == nil { + if rpcFunc == nil || rpcFunc.ws { WriteRPCResponseHTTP(w, types.RPCMethodNotFoundError(request.ID)) return } - if rpcFunc.ws { - WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID, errors.New("Trying to use Websocket method in non-ws context"))) - return - } args, err := jsonParamsToArgsRPC(rpcFunc, request.Params) if err != nil { WriteRPCResponseHTTP(w, types.RPCInvalidParamsError(request.ID, errors.Wrap(err, "Error converting json params to arguments"))) @@ -234,7 +230,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit // Exception for websocket endpoints if rpcFunc.ws { return func(w http.ResponseWriter, r *http.Request) { - WriteRPCResponseHTTP(w, types.RPCInternalError("", errors.New("Trying to use Websocket method in non-ws context"))) + WriteRPCResponseHTTP(w, types.RPCMethodNotFoundError("")) } } // All other endpoints