fix: fail starting Snap if no exports found (#2357)

If no valid exports were found during initial eval of the Snap, fail early instead of potentially throwing misleading errors later. --------- Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
MetaMask · Apr 24, 2024 · 8cc7ad9 · 8cc7ad9
1 parent da52434
commit 8cc7ad9
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 22 deletions.
diff --git a/packages/snaps-controllers/src/services/AbstractExecutionService.test.ts b/packages/snaps-controllers/src/services/AbstractExecutionService.test.ts
@@ -34,9 +34,9 @@ describe('AbstractExecutionService', () => {
     await service.executeSnap({
       snapId: 'TestSnap',
       sourceCode: `
-        console.log('foo');
+        module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     const { streams } = service.getJobs().values().next().value;
@@ -61,9 +61,9 @@ describe('AbstractExecutionService', () => {
     await service.executeSnap({
       snapId: 'TestSnap',
       sourceCode: `
-        console.log('foo');
+      module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     const { streams } = service.getJobs().values().next().value;
@@ -111,9 +111,9 @@ describe('AbstractExecutionService', () => {
     await service.executeSnap({
       snapId: MOCK_SNAP_ID,
       sourceCode: `
-        console.log('foo');
+         module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     await expect(

diff --git a/packages/snaps-controllers/src/services/iframe/IframeExecutionService.test.browser.ts b/packages/snaps-controllers/src/services/iframe/IframeExecutionService.test.browser.ts
@@ -30,9 +30,9 @@ describe('IframeExecutionService', () => {
     const response = await service.executeSnap({
       snapId: 'TestSnap',
       sourceCode: `
-        console.log('foo');
+        module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     expect(response).toBe('OK');

diff --git a/packages/snaps-controllers/src/services/node-js/NodeProcessExecutionService.test.ts b/packages/snaps-controllers/src/services/node-js/NodeProcessExecutionService.test.ts
@@ -20,9 +20,9 @@ describe('NodeProcessExecutionService', () => {
     const response = await service.executeSnap({
       snapId: 'TestSnap',
       sourceCode: `
-        console.log('foo');
+      module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
     expect(response).toBe('OK');
     await service.terminateAllSnaps();
@@ -205,6 +205,7 @@ describe('NodeProcessExecutionService', () => {
     const result = await service.executeSnap({
       snapId,
       sourceCode: `
+        module.exports.onRpcRequest = () => null;
         console.log('foo');
         console.error('bar');
       `,

diff --git a/packages/snaps-controllers/src/services/node-js/NodeThreadExecutionService.test.ts b/packages/snaps-controllers/src/services/node-js/NodeThreadExecutionService.test.ts
@@ -20,9 +20,9 @@ describe('NodeThreadExecutionService', () => {
     const response = await service.executeSnap({
       snapId: 'TestSnap',
       sourceCode: `
-        console.log('foo');
+      module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
     expect(response).toBe('OK');
     await service.terminateAllSnaps();
@@ -206,6 +206,7 @@ describe('NodeThreadExecutionService', () => {
     const result = await service.executeSnap({
       snapId,
       sourceCode: `
+        module.exports.onRpcRequest = () => null;
         console.log('foo');
         console.error('bar');
       `,

diff --git a/packages/snaps-controllers/src/services/webworker/WebWorkerExecutionService.test.browser.ts b/packages/snaps-controllers/src/services/webworker/WebWorkerExecutionService.test.browser.ts
@@ -37,17 +37,17 @@ describe('WebWorkerExecutionService', () => {
     await service.executeSnap({
       snapId: MOCK_SNAP_ID,
       sourceCode: `
-        console.log('foo');
+        module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     await service.executeSnap({
       snapId: MOCK_LOCAL_SNAP_ID,
       sourceCode: `
-        console.log('foo');
+        module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     expect(document.getElementById(WORKER_POOL_ID)).not.toBeNull();
@@ -64,9 +64,9 @@ describe('WebWorkerExecutionService', () => {
     const response = await service.executeSnap({
       snapId: 'TestSnap',
       sourceCode: `
-        console.log('foo');
+        module.exports.onRpcRequest = () => null;
       `,
-      endowments: ['console'],
+      endowments: [],
     });
 
     expect(response).toBe('OK');

diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json
@@ -1,6 +1,6 @@
 {
   "branches": 80,
   "functions": 90.06,
-  "lines": 90.75,
-  "statements": 90.12
+  "lines": 90.76,
+  "statements": 90.13
 }
diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts
@@ -40,6 +40,8 @@ describe('BaseSnapExecutor', () => {
       const CODE = `
         setTimeout(() => { throw new Error('setTimeout executed'); }, 10);
         setInterval(() => { throw new Error('setInterval executed'); }, 10);
+
+        exports.onRpcRequest = () => null;
       `;
 
       const executor = new TestSnapExecutor();
@@ -1242,6 +1244,28 @@ describe('BaseSnapExecutor', () => {
     });
   });
 
+  it("throws if the Snap doesn't export anything", async () => {
+    const CODE = ``;
+
+    const executor = new TestSnapExecutor();
+    await executor.executeSnap(1, MOCK_SNAP_ID, CODE, []);
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      id: 1,
+      error: expect.objectContaining({
+        code: -32603,
+        message: `Error while running snap '${MOCK_SNAP_ID}': Snap has no valid exports.`,
+        data: {
+          cause: expect.objectContaining({
+            code: -32603,
+            message: 'Snap has no valid exports.',
+          }),
+        },
+      }),
+    });
+  });
+
   it('supports onTransaction export', async () => {
     const CODE = `
       module.exports.onTransaction = ({ transaction, chainId, transactionOrigin }) =>

diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts
@@ -461,6 +461,9 @@ export class BaseSnapExecutor {
       }
       return acc;
     }, {});
+
+    // If the Snap has no valid exports after this, fail.
+    assert(Object.keys(data.exports).length > 0, 'Snap has no valid exports.');
   }
 
   /**

diff --git a/packages/snaps-jest/src/internals/server.test.ts b/packages/snaps-jest/src/internals/server.test.ts
@@ -30,7 +30,7 @@ describe('startServer', () => {
               "registry": "https://registry.npmjs.org",
             },
           },
-          "shasum": "D3ANeNZ7C1Ynx0GTP07afj72Jq06Srlq49QZkhICY+E=",
+          "shasum": "uaLwMO39qzKbshqPM6W2Ju7gkO/czuwgNKpjzXRXJj0=",
         },
         "version": "1.0.0",
       }

diff --git a/packages/snaps-jest/src/test-utils/snap/snap.js b/packages/snaps-jest/src/test-utils/snap/snap.js
@@ -1,2 +1,4 @@
 // eslint-disable-next-line no-console
 console.log('Hello, world!');
+
+module.exports.onRpcRequest = () => null;
diff --git a/packages/snaps-jest/src/test-utils/snap/snap.manifest.json b/packages/snaps-jest/src/test-utils/snap/snap.manifest.json
@@ -3,7 +3,7 @@
   "description": "baz",
   "version": "1.0.0",
   "source": {
-    "shasum": "D3ANeNZ7C1Ynx0GTP07afj72Jq06Srlq49QZkhICY+E=",
+    "shasum": "uaLwMO39qzKbshqPM6W2Ju7gkO/czuwgNKpjzXRXJj0=",
     "location": {
       "npm": {
         "filePath": "snap.js",