fix(jsonrpc): recover from panic in handleRequest

jsonrpc/server.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"net/http"
 	"reflect"
+	"runtime/debug"
 	"strings"
 	"sync"
 	"time"
@@ -568,18 +569,41 @@ func isNilOrEmpty(i any) (bool, error) {
 	}
 }
 
-// TODO: add recover() to catch panics from handlers/validators and return a JSON-RPC internal error
-// instead of crashing the HTTP connection
-func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, http.Header, error) {
+func (s *Server) handleRequest(
+	ctx context.Context,
+	req *Request,
+) (res *response, header http.Header, resErr error) {
 	s.logger.Trace("Received request", zap.Object("req", req))
 
-	header := http.Header{}
+	header = http.Header{}
+
+	defer func() {
+		if r := recover(); r != nil {
+			s.logger.Error("Recovered from panic while handling RPC request",
+				zap.String("method", req.Method),
+				zap.Any("panic", r),
+				zap.ByteString("stack", debug.Stack()),
+			)
+			resErr = nil
+			if req.ID == nil { // notification: the spec forbids a response either way
+				res = nil
+				return
+			}
+			res = &response{
+				Version: "2.0",
+				ID:      req.ID,
+				Error:   Err(InternalError, nil),
+			}
+			s.listener.OnRequestFailed(req.Method, res.Error)
+		}
+	}()
+
 	if err := req.isSane(); err != nil {
 		s.logger.Trace("Request sanity check failed", zap.Error(err))
 		return nil, header, err
 	}
 
-	res := &response{
+	res = &response{
 		Version: "2.0",
 		ID:      req.ID,
 	}

jsonrpc/server_test.go

@@ -93,6 +93,9 @@ func TestHandle(t *testing.T) {
 	type validationStruct struct {
 		A int `validate:"min=1"`
 	}
+	type panickingValidationStruct struct {
+		A int `validate:"panics"`
+	}
 	methods := []jsonrpc.Method{
 		{
 			Name:   "method",
@@ -155,6 +158,20 @@ func TestHandle(t *testing.T) {
 				return v["expectedkey"].A, nil
 			},
 		},
+		{
+			Name:   "panics",
+			Params: []jsonrpc.Parameter{},
+			Handler: func() (int, *jsonrpc.Error) {
+				panic("handler panic")
+			},
+		},
+		{
+			Name:   "validationPanics",
+			Params: []jsonrpc.Parameter{{Name: "param"}},
+			Handler: func(v panickingValidationStruct) (int, *jsonrpc.Error) {
+				return v.A, nil
+			},
+		},
 		{
 			Name: "acceptsContext",
 			Handler: func(ctx context.Context) (int, *jsonrpc.Error) {
@@ -198,9 +215,15 @@ func TestHandle(t *testing.T) {
 		},
 	}
 
+	v := validator.New()
+	// register a custom validation tag to simulate a validator panic from validateParam
+	require.NoError(t, v.RegisterValidation("panics", func(fl validator.FieldLevel) bool {
+		panic("validator panic")
+	}))
+
 	listener := CountingEventListener{}
 	server := jsonrpc.NewServer(1, log.NewNopZapLogger()).
-		WithValidator(validator.New()).
+		WithValidator(v).
 		WithListener(&listener)
 	require.NoError(t, server.RegisterMethods(methods...))
 
@@ -544,6 +567,33 @@ func TestHandle(t *testing.T) {
 			req: `{"jsonrpc": "2.0", "method": "multipleOptionalParams", "params": {"param1": 1, "param2": [2, 3], "junk": "junk"}, "id": 1}`,
 			res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"unexpected params: junk"},"id":1}`,
 		},
+
+		"handler panics": {
+			req:              `{"jsonrpc": "2.0", "method": "panics", "params": {}, "id": 1}`,
+			res:              `{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error"},"id":1}`,
+			checkFailedEvent: true,
+		},
+
+		"handler panics as notification": {
+			req: `{"jsonrpc": "2.0", "method": "panics", "params": {}}`,
+			res: ``,
+		},
+
+		"validator panics": {
+			req:              `{"jsonrpc": "2.0", "method": "validationPanics", "params": [{"A": 1}], "id": 1}`,
+			res:              `{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error"},"id":1}`,
+			checkFailedEvent: true,
+		},
+
+		"handler panic mixed with successes in a batch": {
+			req: `[{"jsonrpc" : "2.0", "method" : "method",
+					"params" : { "num" : 5 } , "id" : 5},
+					{"jsonrpc" : "2.0", "method" : "panics",
+					"params" : {} , "id" : 8},
+					{"jsonrpc" : "2.0", "method" : "method",
+					"params" : { "num" : 44 } , "id" : 6}]`,
+			res: `[{"jsonrpc":"2.0","result":{"doubled":10},"id":5},{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error"},"id":8},{"jsonrpc":"2.0","result":{"doubled":88},"id":6}]`,
+		},
 	}
 
 	for desc, test := range tests {