rpc: improve error codes for internal server errors (#25678)

This changes the error code returned by the RPC server in certain situations:

- handler panic: code -32603
- result marshaling error: code -32603
- attempt to subscribe via HTTP: code -32001

In all of the above cases, the server previously returned the default error
code -32000.

Co-authored-by: Nicholas Zhao <nicholas.zhao@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
pull/25729/head
Nicholas 2 years ago committed by GitHub
parent 06151eb581
commit 610cf02c4a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 8
      rpc/client_test.go
  2. 18
      rpc/errors.go
  3. 6
      rpc/handler.go
  4. 5
      rpc/json.go
  5. 2
      rpc/server_test.go
  6. 3
      rpc/service.go
  7. 7
      rpc/testdata/internal-error.js
  8. 14
      rpc/testservice_test.go

@ -82,11 +82,15 @@ func TestClientErrorData(t *testing.T) {
}
// Check code.
// The method handler returns an error value which implements the rpc.Error
// interface, i.e. it has a custom error code. The server returns this error code.
expectedCode := testError{}.ErrorCode()
if e, ok := err.(Error); !ok {
t.Fatalf("client did not return rpc.Error, got %#v", e)
} else if e.ErrorCode() != (testError{}.ErrorCode()) {
t.Fatalf("wrong error code %d, want %d", e.ErrorCode(), testError{}.ErrorCode())
} else if e.ErrorCode() != expectedCode {
t.Fatalf("wrong error code %d, want %d", e.ErrorCode(), expectedCode)
}
// Check data.
if e, ok := err.(DataError); !ok {
t.Fatalf("client did not return rpc.DataError, got %#v", e)

@ -54,9 +54,15 @@ var (
_ Error = new(invalidRequestError)
_ Error = new(invalidMessageError)
_ Error = new(invalidParamsError)
_ Error = new(internalServerError)
)
const defaultErrorCode = -32000
const (
errcodeDefault = -32000
errcodeNotificationsUnsupported = -32001
errcodePanic = -32603
errcodeMarshalError = -32603
)
type methodNotFoundError struct{ method string }
@ -101,3 +107,13 @@ type invalidParamsError struct{ message string }
func (e *invalidParamsError) ErrorCode() int { return -32602 }
func (e *invalidParamsError) Error() string { return e.message }
// internalServerError is used for server errors during request processing.
type internalServerError struct {
code int
message string
}
func (e *internalServerError) ErrorCode() int { return e.code }
func (e *internalServerError) Error() string { return e.message }

@ -48,7 +48,6 @@ import (
// if err := op.wait(...); err != nil {
// h.removeRequestOp(op) // timeout, etc.
// }
//
type handler struct {
reg *serviceRegistry
unsubscribeCb *callback
@ -354,7 +353,10 @@ func (h *handler) handleCall(cp *callProc, msg *jsonrpcMessage) *jsonrpcMessage
// handleSubscribe processes *_subscribe method calls.
func (h *handler) handleSubscribe(cp *callProc, msg *jsonrpcMessage) *jsonrpcMessage {
if !h.allowSubscribe {
return msg.errorResponse(ErrNotificationsUnsupported)
return msg.errorResponse(&internalServerError{
code: errcodeNotificationsUnsupported,
message: ErrNotificationsUnsupported.Error(),
})
}
// Subscription method name is first argument.

@ -104,15 +104,14 @@ func (msg *jsonrpcMessage) errorResponse(err error) *jsonrpcMessage {
func (msg *jsonrpcMessage) response(result interface{}) *jsonrpcMessage {
enc, err := json.Marshal(result)
if err != nil {
// TODO: wrap with 'internal server error'
return msg.errorResponse(err)
return msg.errorResponse(&internalServerError{errcodeMarshalError, err.Error()})
}
return &jsonrpcMessage{Version: vsn, ID: msg.ID, Result: enc}
}
func errorMessage(err error) *jsonrpcMessage {
msg := &jsonrpcMessage{Version: vsn, ID: null, Error: &jsonError{
Code: defaultErrorCode,
Code: errcodeDefault,
Message: err.Error(),
}}
ec, ok := err.(Error)

@ -45,7 +45,7 @@ func TestServerRegisterName(t *testing.T) {
t.Fatalf("Expected service calc to be registered")
}
wantCallbacks := 10
wantCallbacks := 12
if len(svc.callbacks) != wantCallbacks {
t.Errorf("Expected %d callbacks for service 'service', got %d", wantCallbacks, len(svc.callbacks))
}

@ -18,7 +18,6 @@ package rpc
import (
"context"
"errors"
"fmt"
"reflect"
"runtime"
@ -199,7 +198,7 @@ func (c *callback) call(ctx context.Context, method string, args []reflect.Value
buf := make([]byte, size)
buf = buf[:runtime.Stack(buf, false)]
log.Error("RPC method " + method + " crashed: " + fmt.Sprintf("%v\n%s", err, buf))
errRes = errors.New("method handler crashed")
errRes = &internalServerError{errcodePanic, "method handler crashed"}
}
}()
// Run the callback.

@ -0,0 +1,7 @@
// These tests trigger various 'internal error' conditions.
--> {"jsonrpc":"2.0","id":1,"method":"test_marshalError","params": []}
<-- {"jsonrpc":"2.0","id":1,"error":{"code":-32603,"message":"json: error calling MarshalText for type *rpc.MarshalErrObj: marshal error"}}
--> {"jsonrpc":"2.0","id":2,"method":"test_panic","params": []}
<-- {"jsonrpc":"2.0","id":2,"error":{"code":-32603,"message":"method handler crashed"}}

@ -70,6 +70,12 @@ func (testError) Error() string { return "testError" }
func (testError) ErrorCode() int { return 444 }
func (testError) ErrorData() interface{} { return "testError data" }
type MarshalErrObj struct{}
func (o *MarshalErrObj) MarshalText() ([]byte, error) {
return nil, errors.New("marshal error")
}
func (s *testService) NoArgsRets() {}
func (s *testService) Echo(str string, i int, args *echoArgs) echoResult {
@ -114,6 +120,14 @@ func (s *testService) ReturnError() error {
return testError{}
}
func (s *testService) MarshalError() *MarshalErrObj {
return &MarshalErrObj{}
}
func (s *testService) Panic() string {
panic("service panic")
}
func (s *testService) CallMeBack(ctx context.Context, method string, args []interface{}) (interface{}, error) {
c, ok := ClientFromContext(ctx)
if !ok {

Loading…
Cancel
Save