Fix #140 (improve error message) (#142)

- return errors like in C# code (neo-project/neo#587)
- update tests
- small refactoring

pkg/rpc/errors.go

@@ -18,11 +18,7 @@ type (
 )
 
 var (
-	// ErrEmptyParams is CSharp error caused by index out of range
+	errInvalidParams = NewInvalidParamsError("", nil)
-	ErrEmptyParams = newError(-2146233086, http.StatusOK, "Index was out of range. Must be non-negative and less than the size of the collection.\nParameter name: index", "", nil)
-
-	// ErrTypeMismatch is CSharp error caused by type mismatch
-	ErrTypeMismatch = newError(-2146233033, http.StatusOK, "One of the identified items was in an invalid format.", "", nil)
 )
 
 func newError(code int64, httpCode int, message string, data string, cause error) *Error {

pkg/rpc/params.go

@@ -61,19 +61,23 @@ func (p Params) ValueAtAndType(index int, valueType string) (*Param, bool) {
 	return nil, false
 }
 
-func (p Params) Value(index int) (*Param, *Error) {
+// Value returns the param struct for the given
+// index if it exists.
+func (p Params) Value(index int) (*Param, error) {
 	if len(p) <= index {
-		return nil, ErrEmptyParams
+		return nil, errInvalidParams
 	}
 	return &p[index], nil
 }
 
-func (p Params) ValueWithType(index int, valType string) (*Param, *Error) {
+// ValueWithType returns the param struct at the given index if it
+// exists and matches the given type.
+func (p Params) ValueWithType(index int, valType string) (*Param, error) {
 	val, err := p.Value(index)
 	if err != nil {
 		return nil, err
 	} else if val.Type != valType {
-		return nil, ErrTypeMismatch
+		return nil, errInvalidParams
 	}
 	return &p[index], nil
 }

pkg/rpc/server.go

@@ -96,8 +96,10 @@ func (s *Server) methodHandler(w http.ResponseWriter, req *Request, reqParams Pa
 		"params": fmt.Sprintf("%v", reqParams),
 	}).Info("processing rpc request")
 
-	var results interface{}
+	var (
-	var resultsErr *Error
+		results    interface{}
+		resultsErr error
+	)
 
 Methods:
 	switch req.Method {
@@ -106,34 +108,30 @@ Methods:
 
 	case "getblock":
 		var hash util.Uint256
-		var err error
 
-		param, exists := reqParams.ValueAt(0)
+		param, err := reqParams.Value(0)
-		if !exists {
+		if err != nil {
-			err = errors.New("Param at index at 0 doesn't exist")
+			resultsErr = err
-			resultsErr = NewInvalidParamsError(err.Error(), err)
+			break Methods
-			break
 		}
 
 		switch param.Type {
 		case "string":
 			hash, err = util.Uint256DecodeString(param.StringVal)
 			if err != nil {
-				resultsErr = NewInvalidParamsError("Problem decoding block hash", err)
+				resultsErr = errInvalidParams
-				break
+				break Methods
 			}
 		case "number":
 			if !s.validBlockHeight(param) {
-				err = invalidBlockHeightError(0, param.IntVal)
+				resultsErr = errInvalidParams
-				resultsErr = NewInvalidParamsError(err.Error(), err)
 				break Methods
 			}
 
 			hash = s.chain.GetHeaderHash(param.IntVal)
 		case "default":
-			err = errors.New("Expected param at index 0 to be either string or number")
+			resultsErr = errInvalidParams
-			resultsErr = NewInvalidParamsError(err.Error(), err)
+			break Methods
-			break
 		}
 
 		block, err := s.chain.GetBlock(hash)
@@ -147,14 +145,17 @@ Methods:
 		results = s.chain.BlockHeight()
 
 	case "getblockhash":
-		if param, exists := reqParams.ValueAtAndType(0, "number"); exists && s.validBlockHeight(param) {
+		param, err := reqParams.ValueWithType(0, "number")
-			results = s.chain.GetHeaderHash(param.IntVal)
+		if err != nil {
-		} else {
+			resultsErr = err
-			err := invalidBlockHeightError(0, param.IntVal)
+			break Methods
-			resultsErr = NewInvalidParamsError(err.Error(), err)
+		} else if !s.validBlockHeight(param) {
-			break
+			resultsErr = invalidBlockHeightError(0, param.IntVal)
+			break Methods
 		}
 
+		results = s.chain.GetHeaderHash(param.IntVal)
+
 	case "getconnectioncount":
 		results = s.coreServer.PeerCount()
 
@@ -188,21 +189,20 @@ Methods:
 		param, err := reqParams.Value(0)
 		if err != nil {
 			resultsErr = err
-			break
+			break Methods
 		}
 		results = wrappers.ValidateAddress(param.RawValue)
 
 	case "getassetstate":
-		param, errRes := reqParams.ValueWithType(0, "string")
+		param, err := reqParams.ValueWithType(0, "string")
-		if errRes != nil {
+		if err != nil {
-			resultsErr = errRes
+			resultsErr = err
-			break
+			break Methods
 		}
 
 		paramAssetID, err := util.Uint256DecodeString(param.StringVal)
 		if err != nil {
-			err = errors.Wrapf(err, "unable to decode %s to Uint256", param.StringVal)
+			resultsErr = errInvalidParams
-			resultsErr = NewInvalidParamsError(err.Error(), err)
 			break
 		}
 
@@ -218,8 +218,7 @@ Methods:
 		if err != nil {
 			resultsErr = err
 		} else if scriptHash, err := crypto.Uint160DecodeAddress(param.StringVal); err != nil {
-			err = errors.Wrapf(err, "unable to decode %s to Uint160", param.StringVal)
+			resultsErr = errInvalidParams
-			resultsErr = NewInvalidParamsError(err.Error(), err)
 		} else if as := s.chain.GetAccountState(scriptHash); as != nil {
 			results = wrappers.NewAccountState(as)
 		} else {

pkg/rpc/server_test.go

@@ -38,7 +38,7 @@ func TestHandler(t *testing.T) {
 	// setup handler
 	handler := http.HandlerFunc(rpcServer.requestHandler)
 
-	testCases := []struct {
+	var testCases = []struct {
 		rpcCall        string
 		method         string
 		expectedResult string
@@ -51,21 +51,27 @@ func TestHandler(t *testing.T) {
 			"getassetstate_2",
 			`{"jsonrpc":"2.0","result":{"assetId":"0xc56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b","assetType":0,"name":"NEO","amount":"100000000","available":"0","precision":0,"fee":0,"address":"0x0000000000000000000000000000000000000000","owner":"00","admin":"Abf2qMs1pzQb8kYk9RuxtUb9jtRKJVuBJt","issuer":"AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM","expiration":0,"is_frozen":false},"id":1}`},
 
-		{`{"jsonrpc": "2.0", "id": 1, "method": "getassetstate", "params": ["62c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"] }`,
+		{
-			"getassetstate_3",
+			rpcCall:        `{"jsonrpc": "2.0", "id": 1, "method": "getassetstate", "params": ["62c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"] }`,
-			`{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"unable to decode 62c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7 to Uint256: expected string size of 64 got 63"},"id":1}`},
+			method:         "getassetstate_3",
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params"},"id":1}`,
+		},
 
-		{`{"jsonrpc": "2.0", "id": 1, "method": "getassetstate", "params": [123] }`,
+		{
-			"getassetstate_4",
+			rpcCall:        `{"jsonrpc": "2.0", "id": 1, "method": "getassetstate", "params": [123] }`,
-			`{"jsonrpc":"2.0","error":{"code":-2146233033,"message":"One of the identified items was in an invalid format."},"id":1}`},
+			method:         "getassetstate_4",
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params"},"id":1}`,
+		},
 
 		{`{"jsonrpc": "2.0", "id": 1, "method": "getblockhash", "params": [10] }`,
 			"getblockhash_1",
 			`{"jsonrpc":"2.0","result":"0xd69e7a1f62225a35fed91ca578f33447d93fa0fd2b2f662b957e19c38c1dab1e","id":1}`},
 
-		{`{"jsonrpc": "2.0", "id": 1, "method": "getblockhash", "params": [-2] }`,
+		{
-			"getblockhash_2",
+			rpcCall:        `{"jsonrpc": "2.0", "id": 1, "method": "getblockhash", "params": [-2] }`,
-			`{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"Param at index 0 should be greater than or equal to 0 and less then or equal to current block height, got: -2"},"id":1}`},
+			method:         "getblockhash_2",
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error","data":"Internal server error"},"id":1}`,
+		},
 
 		{`{"jsonrpc": "2.0", "id": 1, "method": "getblock", "params": [10] }`,
 			"getblock",
@@ -102,21 +108,21 @@ func TestHandler(t *testing.T) {
 		{
 			rpcCall:        `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": ["AK2nJJpJr6o664CWJKi1QRXjqeic2zR"] }`,
 			method:         "getaccountstate_2",
-			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"unable to decode AK2nJJpJr6o664CWJKi1QRXjqeic2zR to Uint160: invalid base-58 check string: invalid checksum."},"id":1}`,
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params"},"id":1}`,
 		},
 
 		// Bad case, not string
 		{
 			rpcCall:        `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": [123] }`,
 			method:         "getaccountstate_3",
-			expectedResult: `{"jsonrpc":"2.0","error":{"code":-2146233033,"message":"One of the identified items was in an invalid format."},"id":1}`,
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params"},"id":1}`,
 		},
 
 		// Bad case, empty params
 		{
 			rpcCall:        `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": [] }`,
 			method:         "getaccountstate_4",
-			expectedResult: `{"jsonrpc":"2.0","error":{"code":-2146233086,"message":"Index was out of range. Must be non-negative and less than the size of the collection.\nParameter name: index"},"id":1}`,
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params"},"id":1}`,
 		},
 
 		// Good case, valid address
@@ -144,10 +150,9 @@ func TestHandler(t *testing.T) {
 		{
 			rpcCall:        `{ "jsonrpc": "2.0", "id": 1, "method": "validateaddress", "params": [] }`,
 			method:         "validateaddress_4",
-			expectedResult: `{"jsonrpc":"2.0","error":{"code":-2146233086,"message":"Index was out of range. Must be non-negative and less than the size of the collection.\nParameter name: index"},"id":1}`,
+			expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params"},"id":1}`,
 		},
 	}
-
 	for _, tc := range testCases {
 		t.Run(fmt.Sprintf("method: %s, rpc call: %s", tc.method, tc.rpcCall), func(t *testing.T) {