Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #140 (improve error messages) #142

Merged
merged 1 commit into from
Feb 20, 2019
Merged

Fix #140 (improve error messages) #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions pkg/rpc/errors.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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