Skip to content

Commit 7c8a082

Browse files
peteski22peteski22
authored
Improve JSON-RPC error handling with typed errors and response constructors (#595)
* feat: add typed JSON-RPC error handling with sentinel errors * Add JSONRPCResponse constructors for transport layer * Use constructors in transport layer * Update client and tests to use new error handling * Add NewJSONRPCResultResponse constructor and update server usage
1 parent dd0058c commit 7c8a082

19 files changed

+636
-227
lines changed

‎client/client.go‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package client
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"slices"
98
"sync"
@@ -166,7 +165,7 @@ func (c *Client) sendRequest(
166165
}
167166

168167
if response.Error != nil {
169-
return nil, errors.New(response.Error.Message)
168+
return nil, response.Error.AsError()
170169
}
171170

172171
return &response.Result, nil
@@ -524,11 +523,7 @@ func (c *Client) handleSamplingRequestTransport(ctx context.Context, request tra
524523
}
525524

526525
// Create the transport response
527-
response := &transport.JSONRPCResponse{
528-
JSONRPC: mcp.JSONRPC_VERSION,
529-
ID: request.ID,
530-
Result: json.RawMessage(resultBytes),
531-
}
526+
response := transport.NewJSONRPCResultResponse(request.ID, json.RawMessage(resultBytes))
532527

533528
return response, nil
534529
}
@@ -572,22 +567,14 @@ func (c *Client) handleElicitationRequestTransport(ctx context.Context, request
572567
}
573568

574569
// Create the transport response
575-
response := &transport.JSONRPCResponse{
576-
JSONRPC: mcp.JSONRPC_VERSION,
577-
ID: request.ID,
578-
Result: json.RawMessage(resultBytes),
579-
}
570+
response := transport.NewJSONRPCResultResponse(request.ID, resultBytes)
580571

581572
return response, nil
582573
}
583574

584575
func (c *Client) handlePingRequestTransport(ctx context.Context, request transport.JSONRPCRequest) (*transport.JSONRPCResponse, error) {
585576
b, _ := json.Marshal(&mcp.EmptyResult{})
586-
return &transport.JSONRPCResponse{
587-
JSONRPC: mcp.JSONRPC_VERSION,
588-
ID: request.ID,
589-
Result: b,
590-
}, nil
577+
return transport.NewJSONRPCResultResponse(request.ID, b), nil
591578
}
592579

593580
func listByPage[T any](

‎client/elicitation_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,7 @@ func TestClient_Initialize_WithElicitationHandler(t *testing.T) {
155155
}
156156

157157
resultBytes, _ := json.Marshal(result)
158-
return &transport.JSONRPCResponse{
159-
ID: request.ID,
160-
Result: json.RawMessage(resultBytes),
161-
}, nil
158+
return transport.NewJSONRPCResultResponse(request.ID, resultBytes), nil
162159
},
163160
sendNotificationFunc: func(ctx context.Context, notification mcp.JSONRPCNotification) error {
164161
return nil
@@ -183,7 +180,6 @@ func TestClient_Initialize_WithElicitationHandler(t *testing.T) {
183180
Capabilities: mcp.ClientCapabilities{},
184181
},
185182
})
186-
187183
if err != nil {
188184
t.Fatalf("failed to initialize: %v", err)
189185
}

‎client/protocol_negotiation_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,7 @@ func (m *mockProtocolTransport) SendRequest(ctx context.Context, request transpo
3030
return nil, fmt.Errorf("no mock response for method %s", request.Method)
3131
}
3232

33-
return &transport.JSONRPCResponse{
34-
JSONRPC: "2.0",
35-
ID: request.ID,
36-
Result: json.RawMessage(responseStr),
37-
}, nil
33+
return transport.NewJSONRPCResultResponse(request.ID, json.RawMessage(responseStr)), nil
3834
}
3935

4036
func (m *mockProtocolTransport) SendNotification(ctx context.Context, notification mcp.JSONRPCNotification) error {

‎client/sampling_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,10 @@ func TestClient_Initialize_WithSampling(t *testing.T) {
175175
}
176176

177177
// Prepare mock response for initialization
178-
initResponse := &transport.JSONRPCResponse{
179-
JSONRPC: mcp.JSONRPC_VERSION,
180-
ID: mcp.NewRequestId(1),
181-
Result: []byte(`{"protocolVersion":"2024-11-05","capabilities":{"logging":{},"prompts":{},"resources":{},"tools":{}},"serverInfo":{"name":"test-server","version":"1.0.0"}}`),
182-
}
178+
initResponse := transport.NewJSONRPCResultResponse(
179+
mcp.NewRequestId(1),
180+
[]byte(`{"protocolVersion":"2024-11-05","capabilities":{"logging":{},"prompts":{},"resources":{},"tools":{}},"serverInfo":{"name":"test-server","version":"1.0.0"}}`),
181+
)
183182

184183
// Send the response in a goroutine
185184
go func() {

‎client/transport/inprocess.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (c *InProcessTransport) SendRequest(ctx context.Context, request JSONRPCReq
8282
if err != nil {
8383
return nil, fmt.Errorf("failed to marshal response message: %w", err)
8484
}
85-
rpcResp := JSONRPCResponse{}
85+
var rpcResp JSONRPCResponse
8686
err = json.Unmarshal(respByte, &rpcResp)
8787
if err != nil {
8888
return nil, fmt.Errorf("failed to unmarshal response message: %w", err)

‎client/transport/interface.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,13 @@ type JSONRPCRequest struct {
6161
Params any `json:"params,omitempty"`
6262
}
6363

64+
// JSONRPCResponse represents a JSON-RPC 2.0 response message.
65+
// Use NewJSONRPCResultResponse to create a JSONRPCResponse with a result.
66+
// Use NewJSONRPCErrorResponse to create a JSONRPCResponse with an error.
6467
type JSONRPCResponse struct {
65-
JSONRPC string `json:"jsonrpc"`
66-
ID mcp.RequestId `json:"id"`
67-
Result json.RawMessage `json:"result,omitempty"`
68-
Error *struct {
69-
Code int `json:"code"`
70-
Message string `json:"message"`
71-
Data json.RawMessage `json:"data"`
72-
} `json:"error,omitempty"`
68+
JSONRPC string `json:"jsonrpc"`
69+
ID mcp.RequestId `json:"id"`
70+
Result json.RawMessage `json:"result,omitempty"`
71+
Error *mcp.JSONRPCErrorDetails `json:"error,omitempty"`
7372
}
73+

‎client/transport/stdio.go‎

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -402,18 +402,12 @@ func (c *Stdio) handleIncomingRequest(request JSONRPCRequest) {
402402

403403
if handler == nil {
404404
// Send error response if no handler is configured
405-
errorResponse := JSONRPCResponse{
406-
JSONRPC: mcp.JSONRPC_VERSION,
407-
ID: request.ID,
408-
Error: &struct {
409-
Code int `json:"code"`
410-
Message string `json:"message"`
411-
Data json.RawMessage `json:"data"`
412-
}{
413-
Code: mcp.METHOD_NOT_FOUND,
414-
Message: "No request handler configured",
415-
},
416-
}
405+
errorResponse := *NewJSONRPCErrorResponse(
406+
request.ID,
407+
mcp.METHOD_NOT_FOUND,
408+
"No request handler configured",
409+
nil,
410+
)
417411
c.sendResponse(errorResponse)
418412
return
419413
}
@@ -427,37 +421,15 @@ func (c *Stdio) handleIncomingRequest(request JSONRPCRequest) {
427421
// Check if context is already cancelled before processing
428422
select {
429423
case <-ctx.Done():
430-
errorResponse := JSONRPCResponse{
431-
JSONRPC: mcp.JSONRPC_VERSION,
432-
ID: request.ID,
433-
Error: &struct {
434-
Code int `json:"code"`
435-
Message string `json:"message"`
436-
Data json.RawMessage `json:"data"`
437-
}{
438-
Code: mcp.INTERNAL_ERROR,
439-
Message: ctx.Err().Error(),
440-
},
441-
}
424+
errorResponse := *NewJSONRPCErrorResponse(request.ID, mcp.INTERNAL_ERROR, ctx.Err().Error(), nil)
442425
c.sendResponse(errorResponse)
443426
return
444427
default:
445428
}
446429

447430
response, err := handler(ctx, request)
448431
if err != nil {
449-
errorResponse := JSONRPCResponse{
450-
JSONRPC: mcp.JSONRPC_VERSION,
451-
ID: request.ID,
452-
Error: &struct {
453-
Code int `json:"code"`
454-
Message string `json:"message"`
455-
Data json.RawMessage `json:"data"`
456-
}{
457-
Code: mcp.INTERNAL_ERROR,
458-
Message: err.Error(),
459-
},
460-
}
432+
errorResponse := *NewJSONRPCErrorResponse(request.ID, mcp.INTERNAL_ERROR, err.Error(), nil)
461433
c.sendResponse(errorResponse)
462434
return
463435
}

‎client/transport/stdio_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -536,11 +536,7 @@ func TestStdioErrors(t *testing.T) {
536536
t.Cleanup(func() { _ = stdio.Close() })
537537

538538
stdio.SetRequestHandler(func(ctx context.Context, request JSONRPCRequest) (*JSONRPCResponse, error) {
539-
return &JSONRPCResponse{
540-
JSONRPC: "2.0",
541-
ID: request.ID,
542-
Result: json.RawMessage(`"test response"`),
543-
}, nil
539+
return NewJSONRPCResultResponse(request.ID, json.RawMessage(`"test response"`)), nil
544540
})
545541

546542
doneChan := make(chan struct{})

‎client/transport/streamable_http.go‎

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -683,18 +683,12 @@ func (c *StreamableHTTP) handleIncomingRequest(ctx context.Context, request JSON
683683
if handler == nil {
684684
c.logger.Errorf("received request from server but no handler set: %s", request.Method)
685685
// Send method not found error
686-
errorResponse := &JSONRPCResponse{
687-
JSONRPC: "2.0",
688-
ID: request.ID,
689-
Error: &struct {
690-
Code int `json:"code"`
691-
Message string `json:"message"`
692-
Data json.RawMessage `json:"data"`
693-
}{
694-
Code: -32601, // Method not found
695-
Message: fmt.Sprintf("no handler configured for method: %s", request.Method),
696-
},
697-
}
686+
errorResponse := NewJSONRPCErrorResponse(
687+
request.ID,
688+
mcp.METHOD_NOT_FOUND,
689+
fmt.Sprintf("no handler configured for method: %s", request.Method),
690+
nil,
691+
)
698692
c.sendResponseToServer(ctx, errorResponse)
699693
return
700694
}
@@ -715,36 +709,25 @@ func (c *StreamableHTTP) handleIncomingRequest(ctx context.Context, request JSON
715709

716710
// Check for specific sampling-related errors
717711
if errors.Is(err, context.Canceled) {
718-
errorCode = -32800 // Request cancelled
712+
errorCode = mcp.REQUEST_INTERRUPTED
719713
errorMessage = "request was cancelled"
720714
} else if errors.Is(err, context.DeadlineExceeded) {
721-
errorCode = -32800 // Request timeout
715+
errorCode = mcp.REQUEST_INTERRUPTED
722716
errorMessage = "request timed out"
723717
} else {
724718
// Generic error cases
725719
switch request.Method {
726720
case string(mcp.MethodSamplingCreateMessage):
727-
errorCode = -32603 // Internal error
721+
errorCode = mcp.INTERNAL_ERROR
728722
errorMessage = fmt.Sprintf("sampling request failed: %v", err)
729723
default:
730-
errorCode = -32603 // Internal error
724+
errorCode = mcp.INTERNAL_ERROR
731725
errorMessage = err.Error()
732726
}
733727
}
734728

735729
// Send error response
736-
errorResponse := &JSONRPCResponse{
737-
JSONRPC: "2.0",
738-
ID: request.ID,
739-
Error: &struct {
740-
Code int `json:"code"`
741-
Message string `json:"message"`
742-
Data json.RawMessage `json:"data"`
743-
}{
744-
Code: errorCode,
745-
Message: errorMessage,
746-
},
747-
}
730+
errorResponse := NewJSONRPCErrorResponse(request.ID, errorCode, errorMessage, nil)
748731
c.sendResponseToServer(requestCtx, errorResponse)
749732
return
750733
}

‎client/transport/streamable_http_sampling_test.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,7 @@ func TestStreamableHTTP_SamplingFlow(t *testing.T) {
5050

5151
resultBytes, _ := json.Marshal(result)
5252

53-
return &JSONRPCResponse{
54-
JSONRPC: "2.0",
55-
ID: request.ID,
56-
Result: resultBytes,
57-
}, nil
53+
return NewJSONRPCResultResponse(request.ID, resultBytes), nil
5854
})
5955

6056
// Start the client
@@ -360,11 +356,7 @@ func TestStreamableHTTP_ConcurrentSamplingRequests(t *testing.T) {
360356

361357
resultBytes, _ := json.Marshal(result)
362358

363-
return &JSONRPCResponse{
364-
JSONRPC: "2.0",
365-
ID: request.ID,
366-
Result: resultBytes,
367-
}, nil
359+
return NewJSONRPCResultResponse(request.ID, resultBytes), nil
368360
})
369361

370362
// Start the client

0 commit comments

Comments
 (0)