Skip to content

Commit e0942ca

Browse files
renatomaiarenatomaia
committed
fix(jsonrpc): improve error messages reported to the client
- Added specific messages for invalid IDs and unsupported JSON-RPC versions. - Preserved empty-method validation as -32600. - Normalized standard messages to Parse error and Invalid Request. - Added regression assertions for the new messages.
1 parent 4946ec5 commit e0942ca

3 files changed

Lines changed: 39 additions & 24 deletions

File tree

‎internal/jsonrpc/batchcalls_test.go‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ func TestJSONRPCMalformedBatchReturnsParseErrorObject(t *testing.T) {
154154

155155
require.Equal(t, http.StatusOK, rr.Code)
156156
require.Equal(t, "application/json", rr.Header().Get("Content-Type"))
157-
requireRPCError(t, decodeRPCResponse(t, rr.Body.Bytes()), nil, JSONRPC_PARSE_ERROR)
157+
response := decodeRPCResponse(t, rr.Body.Bytes())
158+
requireRPCError(t, response, nil, JSONRPC_PARSE_ERROR)
159+
require.Equal(t, "Parse error", response.Error.Message)
158160
}
159161

160162
func TestJSONRPCMalformedObjectReturnsJSONContentType(t *testing.T) {
@@ -163,7 +165,9 @@ func TestJSONRPCMalformedObjectReturnsJSONContentType(t *testing.T) {
163165

164166
require.Equal(t, http.StatusOK, rr.Code)
165167
require.Equal(t, "application/json", rr.Header().Get("Content-Type"))
166-
requireRPCError(t, decodeRPCResponse(t, rr.Body.Bytes()), nil, JSONRPC_PARSE_ERROR)
168+
response := decodeRPCResponse(t, rr.Body.Bytes())
169+
requireRPCError(t, response, nil, JSONRPC_PARSE_ERROR)
170+
require.Equal(t, "Parse error", response.Error.Message)
167171
}
168172

169173
func TestJSONRPCDiscoverPreservesLargeIntegerLiterals(t *testing.T) {
@@ -236,19 +240,24 @@ func TestJSONRPCBatchStructurallyInvalidElementsDoNotPoisonValidSiblings(t *test
236240

237241
func TestJSONRPCValidationErrorsEchoValidID(t *testing.T) {
238242
s := newBatchTestService()
239-
tests := map[string]string{
240-
"missing method": `{"jsonrpc":"2.0","id":"request-id"}`,
241-
"invalid version": `{"jsonrpc":"1.0","method":"cartesi_getNodeVersion","id":42}`,
243+
tests := map[string]struct {
244+
body string
245+
id any
246+
message string
247+
}{
248+
"missing method": {
249+
body: `{"jsonrpc":"2.0","id":"request-id"}`, id: "request-id", message: "Invalid Request",
250+
},
251+
"invalid version": {
252+
body: `{"jsonrpc":"1.0","method":"cartesi_getNodeVersion","id":42}`, id: float64(42), message: "Unsupported JSON-RPC version",
253+
},
242254
}
243255

244-
for name, body := range tests {
256+
for name, test := range tests {
245257
t.Run(name, func(t *testing.T) {
246-
response := decodeRPCResponse(t, serveRPC(t, s, []byte(body)).Body.Bytes())
247-
expectedID := any("request-id")
248-
if name == "invalid version" {
249-
expectedID = float64(42)
250-
}
251-
requireRPCError(t, response, expectedID, JSONRPC_INVALID_REQUEST)
258+
response := decodeRPCResponse(t, serveRPC(t, s, []byte(test.body)).Body.Bytes())
259+
requireRPCError(t, response, test.id, JSONRPC_INVALID_REQUEST)
260+
require.Equal(t, test.message, response.Error.Message)
252261
})
253262
}
254263
}
@@ -265,6 +274,7 @@ func TestJSONRPCRejectsInvalidIDTypesWithNullID(t *testing.T) {
265274
`{"jsonrpc":"2.0","method":"cartesi_getNodeVersion","id":%s}`, id))
266275
response := decodeRPCResponse(t, serveRPC(t, s, body).Body.Bytes())
267276
requireRPCError(t, response, nil, JSONRPC_INVALID_REQUEST)
277+
require.Equal(t, "Invalid request ID", response.Error.Message)
268278
})
269279
}
270280
}
@@ -470,8 +480,10 @@ func TestJSONRPCBatchReturnsErrorsForIDDRequestsAfterDeadline(t *testing.T) {
470480
requireRPCError(t, responses[4], nil, JSONRPC_INVALID_REQUEST)
471481
requireRPCError(t, responses[5], nil, JSONRPC_TIMEOUT_ERROR)
472482
for i, response := range responses {
473-
if i == 3 || i == 4 {
474-
require.Equal(t, "invalid request", response.Error.Message)
483+
if i == 3 {
484+
require.Equal(t, "Invalid Request", response.Error.Message)
485+
} else if i == 4 {
486+
require.Equal(t, "Invalid request ID", response.Error.Message)
475487
} else {
476488
require.Equal(t, "Request timed out", response.Error.Message)
477489
}

‎internal/jsonrpc/jsonrpc.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,13 @@ func (s *Service) repositoryError(ctx context.Context, message string, err error
218218

219219
func (s *Service) handleRequest(w io.Writer, r *http.Request, req RPCRequest) error {
220220
if !validRPCID(req.ID) {
221-
return writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "invalid request")
221+
return writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "Invalid request ID")
222222
}
223-
if req.JSONRPC != "2.0" || req.Method == "" {
224-
return writeRPCError(w, req.ID, JSONRPC_INVALID_REQUEST, "invalid request")
223+
if req.JSONRPC != "2.0" {
224+
return writeRPCError(w, req.ID, JSONRPC_INVALID_REQUEST, "Unsupported JSON-RPC version")
225+
}
226+
if req.Method == "" {
227+
return writeRPCError(w, req.ID, JSONRPC_INVALID_REQUEST, "Invalid Request")
225228
}
226229
fn, ok := s.handlers[req.Method]
227230
if !ok {
@@ -339,7 +342,7 @@ func (s *Service) handleRPC(w http.ResponseWriter, r *http.Request) {
339342
case '{':
340343
var req RPCRequest
341344
if err := json.Unmarshal(body, &req); err != nil {
342-
s.writeRPCError(w, nil, JSONRPC_PARSE_ERROR, "invalid request")
345+
s.writeRPCError(w, nil, JSONRPC_PARSE_ERROR, "Parse error")
343346
return
344347
}
345348
s.Logger.Info("Dispatching RPC request", "method", truncatedMethod(req.Method))
@@ -350,7 +353,7 @@ func (s *Service) handleRPC(w http.ResponseWriter, r *http.Request) {
350353
// the list-item limit can be checked before dispatching any request.
351354
var reqSeq []json.RawMessage
352355
if err := json.Unmarshal(body, &reqSeq); err != nil {
353-
s.writeRPCError(w, nil, JSONRPC_PARSE_ERROR, "invalid request batch")
356+
s.writeRPCError(w, nil, JSONRPC_PARSE_ERROR, "Parse error")
354357
return
355358
}
356359
if len(reqSeq) == 0 || len(reqSeq) > MAX_BATCH_SIZE {
@@ -382,15 +385,15 @@ func (s *Service) handleRPC(w http.ResponseWriter, r *http.Request) {
382385
case context.DeadlineExceeded:
383386
s.Logger.Warn("RPC method dispatch timeout")
384387
if err := json.Unmarshal(rawReq, &req); err != nil {
385-
responded = s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "invalid request")
388+
responded = s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "Invalid Request")
386389
} else if !validRPCID(req.ID) {
387-
responded = s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "invalid request")
390+
responded = s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "Invalid request ID")
388391
} else {
389392
responded = s.writeRPCError(w, req.ID, JSONRPC_TIMEOUT_ERROR, "Request timed out")
390393
}
391394
default:
392395
if err := json.Unmarshal(rawReq, &req); err != nil {
393-
responded = s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "invalid request")
396+
responded = s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "Invalid Request")
394397
} else {
395398
s.Logger.Debug("Dispatching RPC request", "method", truncatedMethod(req.Method))
396399
responded = s.dispatchOneRequest(w, r, req, budgetResp)
@@ -405,7 +408,7 @@ func (s *Service) handleRPC(w http.ResponseWriter, r *http.Request) {
405408

406409
default:
407410
if json.Valid(body) {
408-
s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "invalid request")
411+
s.writeRPCError(w, nil, JSONRPC_INVALID_REQUEST, "Invalid Request")
409412
} else {
410413
s.writeRPCError(w, nil, JSONRPC_PARSE_ERROR, "Parse error")
411414
}

‎internal/jsonrpc/jsonrpc_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func TestInvalidJSON(t *testing.T) {
6161
var resp RPCResponse
6262
assert.Nil(t, json.Unmarshal(body, &resp))
6363
assert.Equal(t, JSONRPC_PARSE_ERROR, resp.Error.Code)
64-
assert.Equal(t, "invalid request", resp.Error.Message)
64+
assert.Equal(t, "Parse error", resp.Error.Message)
6565
}
6666

6767
// failure: invalid method

0 commit comments

Comments
 (0)