Skip to content

Commit

Permalink
Only count internal errors as failed requests (#1566)
Browse files Browse the repository at this point in the history
request related errors are not really relevant for us and they keep
triggering alerts on production
  • Loading branch information
omerfirmak authored and wojciechos committed Dec 18, 2023
1 parent 6c9630f commit 0f02a79
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
5 changes: 3 additions & 2 deletions jsonrpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,6 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er
args, err := s.buildArguments(ctx, req.Params, calledMethod)
if err != nil {
res.Error = Err(InvalidParams, err.Error())
s.listener.OnRequestFailed(req.Method, err)
return res, nil
}
defer func() {
Expand All @@ -438,7 +437,9 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er

if errAny := tuple[1].Interface(); !isNil(errAny) {
res.Error = errAny.(*Error)
s.listener.OnRequestFailed(req.Method, err)
if res.Error.Code == InternalError {
s.listener.OnRequestFailed(req.Method, err)
}
return res, nil
}
res.Result = tuple[0].Interface()
Expand Down
22 changes: 16 additions & 6 deletions jsonrpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ func TestHandle(t *testing.T) {
return b - a, nil
},
},
{
Name: "errorsInternally",
Params: []jsonrpc.Parameter{},
Handler: func() (int, *jsonrpc.Error) {
return 0, jsonrpc.Err(jsonrpc.InternalError, nil)
},
},
}

listener := CountingEventListener{}
Expand Down Expand Up @@ -198,9 +205,8 @@ func TestHandle(t *testing.T) {
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing/unexpected params in list"},"id":3}`,
},
"too many params": {
req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false, "error message", "too many"] , "id" : 3}`,
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing/unexpected params in list"},"id":3}`,
checkFailedEvent: true,
req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false, "error message", "too many"] , "id" : 3}`,
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing/unexpected params in list"},"id":3}`,
},
"list params": {
req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false, "error message"] , "id" : 3}`,
Expand Down Expand Up @@ -352,9 +358,8 @@ func TestHandle(t *testing.T) {
res: `[{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request","data":"unsupported RPC request version"},"id":5},{"jsonrpc":"2.0","result":{"doubled":88},"id":6}]`,
},
"invalid value in struct": {
req: `{"jsonrpc" : "2.0", "method" : "validation", "params" : [ {"A": 0} ], "id" : 1}`,
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"Key: 'validationStruct.A' Error:Field validation for 'A' failed on the 'min' tag"},"id":1}`,
checkFailedEvent: true,
req: `{"jsonrpc" : "2.0", "method" : "validation", "params" : [ {"A": 0} ], "id" : 1}`,
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"Key: 'validationStruct.A' Error:Field validation for 'A' failed on the 'min' tag"},"id":1}`,
},
"valid value in struct": {
req: `{"jsonrpc" : "2.0", "method" : "validation", "params" : [{"A": 1}], "id" : 1}`,
Expand Down Expand Up @@ -454,6 +459,11 @@ func TestHandle(t *testing.T) {
req: `[1,2,3]`,
res: `[{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request","data":"json: cannot unmarshal number into Go value of type jsonrpc.Request"},"id":null},{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request","data":"json: cannot unmarshal number into Go value of type jsonrpc.Request"},"id":null},{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request","data":"json: cannot unmarshal number into Go value of type jsonrpc.Request"},"id":null}]`,
},
"fails internally": {
req: `{"jsonrpc": "2.0", "method": "errorsInternally", "params": {}, "id": 1}`,
res: `{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal Error"},"id":1}`,
checkFailedEvent: true,
},
}

for desc, test := range tests {
Expand Down

0 comments on commit 0f02a79

Please sign in to comment.