rpc: improve error codes for internal server errors

rpc/client_test.go

@@ -84,8 +84,8 @@ func TestClientErrorData(t *testing.T) {
 	// Check code.
 	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() != internalServerErrorCode {
+		t.Fatalf("wrong error code %d, want %d", e.ErrorCode(), internalServerErrorCode)
 	}
 	// Check data.
 	if e, ok := err.(DataError); !ok {

rpc/errors.go

@@ -35,8 +35,11 @@ func (err HTTPError) Error() string {
 
 // Error wraps RPC errors, which contain an error code in addition to the message.
 type Error interface {
-	Error() string  // returns the message
-	ErrorCode() int // returns the code
+	Error() string // returns the message
+
+	// returns the code following JSON-RPC 2.0 spec
+	// https://www.jsonrpc.org/specification
+	ErrorCode() int
 }
 
 // A DataError contains some data in addition to the error message.
@@ -54,9 +57,14 @@ var (
 	_ Error = new(invalidRequestError)
 	_ Error = new(invalidMessageError)
 	_ Error = new(invalidParamsError)
+	_ Error = new(internalServerError)
+)
+
+const (
+	defaultErrorCode = -32000
+	internalServerErrorCode = -32603
 )
 
-const defaultErrorCode = -32000
 
 type methodNotFoundError struct{ method string }
 
@@ -101,3 +109,18 @@ type invalidParamsError struct{ message string }
 func (e *invalidParamsError) ErrorCode() int { return -32602 }
 
 func (e *invalidParamsError) Error() string { return e.message }
+
+type internalServerError struct{ cause error }
+
+func (e *internalServerError) ErrorCode() int { return internalServerErrorCode }
+
+func (e *internalServerError) Error() string {
+	return fmt.Sprintf("internal server error caused by %s", e.cause.Error())
+}
+
+func (e *internalServerError) ErrorData() interface{} {
+	if dataErr, ok := e.cause.(DataError); ok {
+		return dataErr.ErrorData()
+	}
+	return nil
+}

rpc/handler.go

@@ -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,7 @@ 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{cause: ErrNotificationsUnsupported})
 	}
 
 	// Subscription method name is first argument.
@@ -388,7 +387,7 @@ func (h *handler) handleSubscribe(cp *callProc, msg *jsonrpcMessage) *jsonrpcMes
 func (h *handler) runMethod(ctx context.Context, msg *jsonrpcMessage, callb *callback, args []reflect.Value) *jsonrpcMessage {
 	result, err := callb.call(ctx, msg.Method, args)
 	if err != nil {
-		return msg.errorResponse(err)
+		return msg.errorResponse(&internalServerError{cause: err})
 	}
 	return msg.response(result)
 }

rpc/json.go

@@ -104,8 +104,7 @@ 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{cause: err})
 	}
 	return &jsonrpcMessage{Version: vsn, ID: msg.ID, Result: enc}
 }

rpc/server_test.go

@@ -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))
 	}

rpc/testservice_test.go

@@ -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 {

rpc/websocket_test.go

@@ -25,6 +25,7 @@ import (
 	"net/http/httptest"
 	"net/http/httputil"
 	"net/url"
+	"reflect"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -227,6 +228,62 @@ func TestClientWebsocketLargeMessage(t *testing.T) {
 	}
 }
 
+func TestClientWebsocketInternalMarshalError(t *testing.T) {
+	var (
+		srv     = newTestServer()
+		httpsrv = httptest.NewServer(srv.WebsocketHandler(nil))
+		wsURL   = "ws:" + strings.TrimPrefix(httpsrv.URL, "http:")
+	)
+	defer srv.Stop()
+	defer httpsrv.Close()
+
+	c, err := DialWebsocket(context.Background(), wsURL, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var obj MarshalErrObj
+	err = c.Call(&obj, "test_marshalError")
+	if err == nil {
+		t.Fatal("test_marshalError call should return error")
+	}
+	jsonerror, ok := err.(*jsonError)
+	if !ok {
+		t.Fatalf("test_marshalError should reutrn jsonError, but %v found", reflect.TypeOf(err))
+	}
+	if jsonerror.Code != internalServerErrorCode {
+		t.Errorf("wrong error code %d, %d expected", jsonerror.Code, internalServerErrorCode)
+	}
+}
+
+func TestClientWebsocketInternalRPCPanic(t *testing.T) {
+	var (
+		srv     = newTestServer()
+		httpsrv = httptest.NewServer(srv.WebsocketHandler(nil))
+		wsURL   = "ws:" + strings.TrimPrefix(httpsrv.URL, "http:")
+	)
+	defer srv.Stop()
+	defer httpsrv.Close()
+
+	c, err := DialWebsocket(context.Background(), wsURL, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var r string
+	err = c.Call(&r, "test_panic")
+	if err == nil {
+		t.Fatal("test_panic call should return error")
+	}
+	jsonerror, ok := err.(*jsonError)
+	if !ok {
+		t.Fatalf("test_panic should reutrn jsonError, but %v found", reflect.TypeOf(err))
+	}
+	if jsonerror.Code != internalServerErrorCode {
+		t.Errorf("wrong error code %d, %d expected", jsonerror.Code, internalServerErrorCode)
+	}
+}
+
 func TestClientWebsocketSevered(t *testing.T) {
 	t.Parallel()