Merge pull request #521 from tendermint/json-rpc-patch

updated json response to match spec by @davebryson

CHANGELOG.md

@@ -32,6 +32,7 @@ BUG FIXES:
 
 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)
 

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
 }

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
 	}
@@ -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
+	if response.Error != nil {
-	if errorStr != "" {
+		return nil, errors.Errorf("Response error: %v", response.Error.Message)
-		return nil, errors.Errorf("Response error: %v", errorStr)
 	}
 	// unmarshal the RawMessage into the result
 	err = json.Unmarshal(*response.Result, result)

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
 	}
@@ -422,8 +422,8 @@ func (c *WSClient) readRoutine() {
 			c.ErrorsCh <- err
 			continue
 		}
-		if response.Error != "" {
+		if response.Error != nil {
-			c.ErrorsCh <- errors.Errorf(response.Error)
+			c.ErrorsCh <- errors.New(response.Error.Message)
 			continue
 		}
 		c.Logger.Info("got response", "resp", response.Result)

rpc/lib/server/handlers.go

@@ -110,35 +110,36 @@ 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("", 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 {
-			WriteRPCResponseHTTPError(w, http.StatusNotFound, types.NewRPCResponse(request.ID, nil, fmt.Sprintf("Invalid JSONRPC endpoint %s", r.URL.Path)))
+			WriteRPCResponseHTTP(w, types.RPCInvalidRequestError(request.ID, errors.Errorf("Path %s is invalid", r.URL.Path)))
 			return
 		}
 		rpcFunc := funcMap[request.Method]
-		if rpcFunc == nil {
+		if rpcFunc == nil || rpcFunc.ws {
-			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))
 			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, 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 {
-			WriteRPCResponseHTTPError(w, http.StatusInternalServerError, types.NewRPCResponse(request.ID, result, err.Error()))
+			WriteRPCResponseHTTP(w, types.RPCInternalError(request.ID, err))
 			return
 		}
-		WriteRPCResponseHTTP(w, types.NewRPCResponse(request.ID, result, ""))
+		WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse(request.ID, result))
 	}
 }
 
@@ -229,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) {
-			WriteRPCResponseHTTPError(w, http.StatusMethodNotAllowed, types.NewRPCResponse("", nil, "This RPC method is only for websockets"))
+			WriteRPCResponseHTTP(w, types.RPCMethodNotFoundError(""))
 		}
 	}
 	// All other endpoints
@@ -237,17 +238,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("", 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 {
-			WriteRPCResponseHTTPError(w, http.StatusInternalServerError, types.NewRPCResponse("", nil, err.Error()))
+			WriteRPCResponseHTTP(w, types.RPCInternalError("", err))
 			return
 		}
-		WriteRPCResponseHTTP(w, types.NewRPCResponse("", result, ""))
+		WriteRPCResponseHTTP(w, types.NewRPCSuccessResponse("", result))
 	}
 }
 
@@ -509,8 +510,13 @@ 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.RPCParseError("", errors.Wrap(err, "Error unmarshaling request")))
-				wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, nil, errStr))
+				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
 			}
 
@@ -518,7 +524,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 +535,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, errors.Wrap(err, "Error converting json params to arguments")))
 				continue
 			}
 			returns := rpcFunc.f.Call(args)
@@ -539,10 +545,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, err))
 				continue
 			} else {
-				wsc.WriteRPCResponse(types.NewRPCResponse(request.ID, result, ""))
+				wsc.WriteRPCResponse(types.NewRPCSuccessResponse(request.ID, result))
 				continue
 			}
 

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.NewRPCResponse("", nil, fmt.Sprintf("Internal Server Error: %v", e)))
+					WriteRPCResponseHTTP(rww, types.RPCInternalError("", e.(error)))
 				}
 			}
 

rpc/lib/types/types.go

@@ -5,9 +5,14 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	events "github.com/tendermint/tmlibs/events"
 )
 
+//----------------------------------------
+// REQUEST
+
 type RPCRequest struct {
 	JSONRPC string           `json:"jsonrpc"`
 	ID      string           `json:"id"`
@@ -47,42 +52,77 @@ 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"`
 	ID      string           `json:"id"`
-	Result  *json.RawMessage `json:"result"`
+	Result  *json.RawMessage `json:"result,omitempty"`
-	Error   string           `json:"error"`
+	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)
+		js, err := json.Marshal(res)
-		if err2 == nil {
+		if err != nil {
-			rawMsg := json.RawMessage(js)
+			return RPCInternalError(id, errors.Wrap(err, "Error marshalling response"))
-			raw = &rawMsg
-		} else {
-			err = err2.Error()
 		}
+		rawMsg := json.RawMessage(js)
+		raw = &rawMsg
 	}
+
+	return RPCResponse{JSONRPC: "2.0", ID: id, Result: raw}
+}
+
+func NewRPCErrorResponse(id string, code int, msg string, data string) RPCResponse {
 	return RPCResponse{
 		JSONRPC: "2.0",
 		ID:      id,
-		Result:  raw,
+		Error:   &RPCError{Code: code, Message: msg, Data: data},
-		Error:   err,
 	}
 }
 
 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)
 	}
 }
 
+func RPCParseError(id string, err error) RPCResponse {
+	return NewRPCErrorResponse(id, -32700, "Parse error. Invalid JSON", err.Error())
+}
+
+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", "")
+}
+
+func RPCInvalidParamsError(id string, err error) RPCResponse {
+	return NewRPCErrorResponse(id, -32602, "Invalid params", err.Error())
+}
+
+func RPCInternalError(id string, err error) RPCResponse {
+	return NewRPCErrorResponse(id, -32603, "Internal error", err.Error())
+}
+
+func RPCServerError(id string, err error) RPCResponse {
+	return NewRPCErrorResponse(id, -32000, "Server error", err.Error())
+}
+
 //----------------------------------------
 
 // *wsConnection implements this interface.
@@ -100,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

rpc/lib/types/types_test.go

@@ -0,0 +1,32 @@
+package rpctypes
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/pkg/errors"
+	"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", errors.New("Hello world"))
+	e, _ := json.Marshal(d)
+	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))
+}

test/app/counter_test.sh

@@ -29,33 +29,36 @@ function getCode() {
 function sendTx() {
 	TX=$1
 	if [[ "$GRPC_BROADCAST_TX" == "" ]]; then
-		RESPONSE=`curl -s localhost:46657/broadcast_tx_commit?tx=0x$TX`
+		RESPONSE=$(curl -s localhost:46657/broadcast_tx_commit?tx=0x"$TX")
-		ERROR=`echo $RESPONSE | jq .error`
+		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`
+		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_RESPONSE=$(echo "$RESPONSE" | jq '.deliver_tx')
-	APPEND_TX_CODE=`getCode "$APPEND_TX_RESPONSE"`
+	APPEND_TX_CODE=$(getCode "$APPEND_TX_RESPONSE")
-	CHECK_TX_RESPONSE=`echo $RESPONSE | jq .check_tx`
+	CHECK_TX_RESPONSE=$(echo "$RESPONSE" | jq '.check_tx')
-	CHECK_TX_CODE=`getCode "$CHECK_TX_RESPONSE"`
+	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