fix: JSON-RPC server returns spec-compliant errors (#2590)

Fix #2510 

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [ ] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [x] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).

yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts

@@ -56,9 +56,9 @@ export async function defaultFetch(
   }
   if (!resp.ok) {
     if (noRetry) {
-      throw new NoRetryError(responseJson.error);
+      throw new NoRetryError(responseJson.error.message);
     } else {
-      throw new Error(responseJson.error);
+      throw new Error(responseJson.error.message);
     }
   }
 

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',
+  });
+});

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,
+              },
+            };
           }
         }
       });