Fix snaps-controllers and snaps-execution-environments tests (#1835)

This fixes the Snaps controllers and execution environments tests on #1818.
MetaMask · Oct 11, 2023 · 8c9ba92 · 8c9ba92
1 parent 3c6782f
commit 8c9ba92
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 28 deletions.
diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json
@@ -1,5 +1,5 @@
 {
-  "branches": 89.28,
+  "branches": 89.39,
   "functions": 95.6,
   "lines": 96.97,
   "statements": 96.64

diff --git a/packages/snaps-controllers/src/services/AbstractExecutionService.ts b/packages/snaps-controllers/src/services/AbstractExecutionService.ts
@@ -9,7 +9,12 @@ import type {
   JsonRpcRequest,
   PendingJsonRpcResponse,
 } from '@metamask/utils';
-import { Duration, isJsonRpcNotification, isObject } from '@metamask/utils';
+import {
+  Duration,
+  hasProperty,
+  isJsonRpcNotification,
+  isObject,
+} from '@metamask/utils';
 import { createStreamMiddleware } from 'json-rpc-middleware-stream';
 import { nanoid } from 'nanoid';
 import { pipeline } from 'stream';
@@ -47,6 +52,10 @@ export type Job<WorkerType> = {
   worker: WorkerType;
 };
 
+export class ExecutionEnvironmentError extends Error {
+  cause?: Json;
+}
+
 export abstract class AbstractExecutionService<WorkerType>
   implements ExecutionService
 {
@@ -373,12 +382,23 @@ export abstract class AbstractExecutionService<WorkerType>
     }
 
     log('Parent: Sending Command', message);
-    const response: PendingJsonRpcResponse<Json> =
-      // eslint-disable-next-line @typescript-eslint/await-thenable
-      await job.rpcEngine.handle(message);
+    const response: PendingJsonRpcResponse<Json> = await job.rpcEngine.handle(
+      message,
+    );
+
     if (response.error) {
-      throw new Error(response.error.message);
+      const error = new ExecutionEnvironmentError(response.error.message);
+      if (
+        isObject(response.error.data) &&
+        hasProperty(response.error.data, 'cause') &&
+        response.error.data.cause !== null
+      ) {
+        error.cause = response.error.data.cause;
+      }
+
+      throw error;
     }
+
     return response.result;
   }
 

diff --git a/packages/snaps-controllers/src/services/browser.test.ts b/packages/snaps-controllers/src/services/browser.test.ts
@@ -8,6 +8,7 @@ describe('browser entrypoint', () => {
     'OffscreenExecutionService',
     'WebWorkerExecutionService',
     'ProxyPostMessageStream',
+    'ExecutionEnvironmentError',
   ];
 
   it('entrypoint has expected exports', () => {

diff --git a/packages/snaps-controllers/src/services/node/NodeProcessExecutionService.test.ts b/packages/snaps-controllers/src/services/node/NodeProcessExecutionService.test.ts
@@ -2,6 +2,7 @@ import type { SnapId } from '@metamask/snaps-utils';
 import { HandlerType } from '@metamask/snaps-utils';
 
 import { createService, MOCK_BLOCK_NUMBER } from '../../test-utils';
+import { ExecutionEnvironmentError } from '../AbstractExecutionService';
 import type { SnapErrorJson } from '../ExecutionService';
 import { NodeProcessExecutionService } from './NodeProcessExecutionService';
 
@@ -47,7 +48,7 @@ describe('NodeProcessExecutionService', () => {
   });
 
   it('can handle errors in request handler', async () => {
-    expect.assertions(1);
+    expect.assertions(2);
     const { service } = createService(NodeProcessExecutionService);
     const snapId = 'TestSnap';
     await service.executeSnap({
@@ -58,8 +59,8 @@ describe('NodeProcessExecutionService', () => {
       endowments: [],
     });
 
-    await expect(
-      service.handleRpcRequest(snapId, {
+    const result = await service
+      .handleRpcRequest(snapId, {
         origin: 'fooOrigin',
         handler: ON_RPC_REQUEST,
         request: {
@@ -68,8 +69,18 @@ describe('NodeProcessExecutionService', () => {
           params: {},
           id: 1,
         },
-      }),
-    ).rejects.toThrow('foobar');
+      })
+      .catch((error) => error);
+
+    expect(result).toBeInstanceOf(ExecutionEnvironmentError);
+
+    // eslint-disable-next-line jest/prefer-strict-equal
+    expect((result as ExecutionEnvironmentError).cause).toEqual({
+      // TODO: Unwrap errors, and change this to the actual error message.
+      message: 'foobar',
+      stack: expect.any(String),
+    });
+
     await service.terminateAllSnaps();
   });
 
@@ -126,9 +137,10 @@ describe('NodeProcessExecutionService', () => {
       code: -32603,
       data: {
         snapId: 'TestSnap',
-        stack: expect.any(String),
+        stack: expect.stringContaining('Error: random error inside'),
       },
-      message: 'random error inside',
+      // TODO: Unwrap errors, and change this to the actual error message.
+      message: 'Execution Environment Error',
     });
 
     await service.terminateAllSnaps();

diff --git a/packages/snaps-controllers/src/services/node/NodeThreadExecutionService.test.ts b/packages/snaps-controllers/src/services/node/NodeThreadExecutionService.test.ts
@@ -2,6 +2,7 @@ import type { SnapId } from '@metamask/snaps-utils';
 import { HandlerType } from '@metamask/snaps-utils';
 
 import { createService, MOCK_BLOCK_NUMBER } from '../../test-utils';
+import { ExecutionEnvironmentError } from '../AbstractExecutionService';
 import type { SnapErrorJson } from '../ExecutionService';
 import { NodeThreadExecutionService } from './NodeThreadExecutionService';
 
@@ -47,7 +48,7 @@ describe('NodeThreadExecutionService', () => {
   });
 
   it('can handle errors in request handler', async () => {
-    expect.assertions(1);
+    expect.assertions(2);
     const { service } = createService(NodeThreadExecutionService);
     const snapId = 'TestSnap';
     await service.executeSnap({
@@ -58,8 +59,8 @@ describe('NodeThreadExecutionService', () => {
       endowments: [],
     });
 
-    await expect(
-      service.handleRpcRequest(snapId, {
+    const result = await service
+      .handleRpcRequest(snapId, {
         origin: 'fooOrigin',
         handler: ON_RPC_REQUEST,
         request: {
@@ -68,8 +69,18 @@ describe('NodeThreadExecutionService', () => {
           params: {},
           id: 1,
         },
-      }),
-    ).rejects.toThrow('foobar');
+      })
+      .catch((error) => error);
+
+    expect(result).toBeInstanceOf(ExecutionEnvironmentError);
+
+    // eslint-disable-next-line jest/prefer-strict-equal
+    expect((result as ExecutionEnvironmentError).cause).toEqual({
+      // TODO: Unwrap errors, and change this to the actual error message.
+      message: 'foobar',
+      stack: expect.any(String),
+    });
+
     await service.terminateAllSnaps();
   });
 
@@ -126,9 +137,10 @@ describe('NodeThreadExecutionService', () => {
       code: -32603,
       data: {
         snapId: 'TestSnap',
-        stack: expect.any(String),
+        stack: expect.stringContaining('Error: random error inside'),
       },
-      message: 'random error inside',
+      // TODO: Unwrap errors, and change this to the actual error message.
+      message: 'Execution Environment Error',
     });
 
     await service.terminateAllSnaps();

diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json
@@ -1,6 +1,6 @@
 {
-  "branches": 80.41,
+  "branches": 79.72,
   "functions": 91.91,
-  "lines": 91.41,
-  "statements": 91.12
+  "lines": 91.08,
+  "statements": 90.8
 }
diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts
@@ -541,7 +541,8 @@ describe('BaseSnapExecutor', () => {
       jsonrpc: '2.0',
       error: {
         code: -32603,
-        message: expect.stringContaining('undefined'),
+        // TODO: Unwrap errors, and change this to the actual error message.
+        message: 'Execution Environment Error',
         data: expect.any(Object),
       },
       id: 2,
@@ -1022,7 +1023,8 @@ describe('BaseSnapExecutor', () => {
             stack: error.stack,
             snapId: MOCK_SNAP_ID,
           },
-          message: error.message,
+          // TODO: Unwrap errors, and change this to the actual error message.
+          message: 'Execution Environment Error',
         },
       },
     });
@@ -1082,7 +1084,8 @@ describe('BaseSnapExecutor', () => {
             stack: error.stack,
             snapId: MOCK_SNAP_ID,
           },
-          message: error.message,
+          // TODO: Unwrap errors, and change this to the actual error message.
+          message: 'Execution Environment Error',
         },
       },
     });
@@ -1388,7 +1391,8 @@ describe('BaseSnapExecutor', () => {
       error: {
         code: -32603,
         data: expect.anything(),
-        message: expect.stringContaining('undefined'),
+        // TODO: Unwrap errors, and change this to the actual error message.
+        message: 'Execution Environment Error',
       },
     });
 
@@ -1744,7 +1748,8 @@ describe('BaseSnapExecutor', () => {
       error: {
         code: -32603,
         data: expect.any(Object),
-        message: 'JSON-RPC responses must be JSON serializable objects.',
+        // TODO: Unwrap errors, and change this to the actual error message.
+        message: 'Execution Environment Error',
       },
     });
   });