Skip to content

Commit

Permalink
Fix nspcc-dev#140 (improve error message)
Browse files Browse the repository at this point in the history 
- return errors like in C# code (neo-project/neo#587)
- update tests
- small refactoring
  • Loading branch information
@im-kulikov
im-kulikov committed Feb 19, 2019
1 parent 9c24bf9 commit 09acaf9
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 54 deletions.
6 changes: 1 addition & 5 deletions pkg/rpc/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ type (
)

var (
// ErrEmptyParams is CSharp error caused by index out of range
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)
errInvalidParams = NewInvalidParamsError("", nil)
)

func newError(code int64, httpCode int, message string, data string, cause error) *Error {
Expand Down
12 changes: 8 additions & 4 deletions pkg/rpc/params.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
59 changes: 29 additions & 30 deletions pkg/rpc/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 resultsErr *Error
var (
results interface{}
resultsErr error
)

Methods:
switch req.Method {
Expand All @@ -106,34 +108,30 @@ Methods:

case "getblock":
var hash util.Uint256
var err error

param, exists := reqParams.ValueAt(0)
if !exists {
err = errors.New("Param at index at 0 doesn't exist")
resultsErr = NewInvalidParamsError(err.Error(), err)
break
param, err := reqParams.Value(0)
if err != nil {
resultsErr = err
break Methods
}

switch param.Type {
case "string":
hash, err = util.Uint256DecodeString(param.StringVal)
if err != nil {
resultsErr = NewInvalidParamsError("Problem decoding block hash", err)
break
resultsErr = errInvalidParams
break Methods
}
case "number":
if !s.validBlockHeight(param) {
err = invalidBlockHeightError(0, param.IntVal)
resultsErr = NewInvalidParamsError(err.Error(), err)
resultsErr = errInvalidParams
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 = NewInvalidParamsError(err.Error(), err)
break
resultsErr = errInvalidParams
break Methods
}

block, err := s.chain.GetBlock(hash)
Expand All @@ -147,14 +145,17 @@ Methods:
results = s.chain.BlockHeight()

case "getblockhash":
if param, exists := reqParams.ValueAtAndType(0, "number"); exists && s.validBlockHeight(param) {
results = s.chain.GetHeaderHash(param.IntVal)
} else {
err := invalidBlockHeightError(0, param.IntVal)
resultsErr = NewInvalidParamsError(err.Error(), err)
break
param, err := reqParams.ValueWithType(0, "number")
if err != nil {
resultsErr = err
break Methods
} else if !s.validBlockHeight(param) {
resultsErr = invalidBlockHeightError(0, param.IntVal)
break Methods
}

results = s.chain.GetHeaderHash(param.IntVal)

case "getconnectioncount":
results = s.coreServer.PeerCount()

Expand Down Expand Up @@ -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")
if errRes != nil {
resultsErr = errRes
break
param, err := reqParams.ValueWithType(0, "string")
if err != nil {
resultsErr = err
break Methods
}

paramAssetID, err := util.Uint256DecodeString(param.StringVal)
if err != nil {
err = errors.Wrapf(err, "unable to decode %s to Uint256", param.StringVal)
resultsErr = NewInvalidParamsError(err.Error(), err)
resultsErr = errInvalidParams
break
}

Expand All @@ -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 = NewInvalidParamsError(err.Error(), err)
resultsErr = errInvalidParams
} else if as := s.chain.GetAccountState(scriptHash); as != nil {
results = wrappers.NewAccountState(as)
} else {
Expand Down
35 changes: 20 additions & 15 deletions pkg/rpc/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
`{"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}`},
{
rpcCall: `{"jsonrpc": "2.0", "id": 1, "method": "getassetstate", "params": ["62c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"] }`,
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",
`{"jsonrpc":"2.0","error":{"code":-2146233033,"message":"One of the identified items was in an invalid format."},"id":1}`},
{
rpcCall: `{"jsonrpc": "2.0", "id": 1, "method": "getassetstate", "params": [123] }`,
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",
`{"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}`},
{
rpcCall: `{"jsonrpc": "2.0", "id": 1, "method": "getblockhash", "params": [-2] }`,
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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {

Expand Down

0 comments on commit 09acaf9

Please sign in to comment.