fix: JSON-RPC server returns spec-compliant errors

AztecProtocol · Oct 3, 2023 · 50e4ec3 · 50e4ec3
1 parent 10b30e0
commit 50e4ec3
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 6 deletions.
diff --git a/yarn-project/foundation/src/json-rpc/server/json_rpc_server.test.ts b/yarn-project/foundation/src/json-rpc/server/json_rpc_server.test.ts
@@ -22,3 +22,30 @@ test('test an RPC function with an array of classes', async () => {
   expect(response.status).toBe(200);
   expect(response.text).toBe(JSON.stringify({ result: [{ data: 'a' }, { data: 'b' }, { data: 'c' }] }));
 });
+
+test('test invalid JSON', async () => {
+  const server = new JsonRpcServer(new TestState([]), { TestNote }, {}, false);
+  const response = await request(server.getApp().callback()).post('/').send('{');
+  expect(response.status).toBe(400);
+  expect(response.body).toEqual({
+    error: { code: -32700, message: 'Parse error' },
+    id: null,
+    jsonrpc: '2.0',
+  });
+});
+
+test('invalid method', async () => {
+  const server = new JsonRpcServer(new TestState([]), { TestNote }, {}, false);
+  const response = await request(server.getApp().callback()).post('/').send({
+    jsonrpc: '2.0',
+    method: 'invalid',
+    params: [],
+    id: 42,
+  });
+  expect(response.status).toBe(400);
+  expect(response.body).toEqual({
+    error: { code: -32601, message: 'Method not found' },
+    id: 42,
+    jsonrpc: '2.0',
+  });
+});
diff --git a/yarn-project/foundation/src/json-rpc/server/json_rpc_server.ts b/yarn-project/foundation/src/json-rpc/server/json_rpc_server.ts
@@ -39,14 +39,34 @@ export class JsonRpcServer {
         await next();
       } catch (err: any) {
         this.log.error(err);
-        ctx.status = 400;
-        ctx.body = { error: err.message };
+        if (err instanceof SyntaxError) {
+          ctx.status = 400;
+          ctx.body = {
+            jsonrpc: '2.0',
+            id: null,
+            error: {
+              code: -32700,
+              message: 'Parse error',
+            },
+          };
+        } else {
+          ctx.status = 500;
+          ctx.body = {
+            jsonrpc: '2.0',
+            id: null,
+            error: {
+              code: -32603,
+              message: 'Internal error',
+            },
+          };
+        }
       }
     };
     const app = new Koa();
     app.on('error', error => {
       this.log.error(`Error on API handler: ${error}`);
     });
+    app.use(exceptionHandler);
     app.use(compress({ br: false } as any));
     app.use(
       bodyParser({
@@ -56,7 +76,6 @@ export class JsonRpcServer {
       }),
     );
     app.use(cors());
-    app.use(exceptionHandler);
     app.use(router.routes());
     app.use(router.allowedMethods());
 
@@ -98,7 +117,15 @@ export class JsonRpcServer {
             // Propagate the error message to the client. Plenty of the errors are expected to occur (e.g. adding
             // a duplicate recipient) so this is necessary.
             ctx.status = 400;
-            ctx.body = { error: err.message };
+            ctx.body = {
+              jsonrpc,
+              id,
+              error: {
+                // TODO assign error codes - https://github.com/AztecProtocol/aztec-packages/issues/2633
+                code: -32000,
+                message: err.message,
+              },
+            };
           }
         });
       }
@@ -113,7 +140,14 @@ export class JsonRpcServer {
           this.disallowedMethods.includes(method)
         ) {
           ctx.status = 400;
-          ctx.body = { error: `Invalid method name: ${method}` };
+          ctx.body = {
+            jsonrpc,
+            id,
+            error: {
+              code: -32601,
+              message: 'Method not found',
+            },
+          };
         } else {
           try {
             const result = await this.proxy.call(method, params);
@@ -127,7 +161,15 @@ export class JsonRpcServer {
             // Propagate the error message to the client. Plenty of the errors are expected to occur (e.g. adding
             // a duplicate recipient) so this is necessary.
             ctx.status = 400;
-            ctx.body = { error: err.message };
+            ctx.body = {
+              jsonrpc,
+              id,
+              error: {
+                // TODO assign error codes - https://github.com/AztecProtocol/aztec-packages/issues/2633
+                code: -32000,
+                message: err.message,
+              },
+            };
           }
         }
       });