fix(ses): avoid cache corruption when execute() throws

* fix(ses): avoid cache corruption when execute() throws * test(compartment-mapper): cover catching require errors * fix(ses): ensure execute() runs once and error thrown from it is persisted by existing memoization * fix(ses): protect error caching from falsy throws
endojs · Apr 5, 2022 · 1d9c17b · 1d9c17b
1 parent ce0d672
commit 1d9c17b
Show file tree

Hide file tree

Showing 10 changed files with 73 additions and 9 deletions.
diff --git a/...es/compartment-mapper/test/fixtures-error-handling/node_modules/catch/README.md b/...es/compartment-mapper/test/fixtures-error-handling/node_modules/catch/README.md
diff --git a/packages/compartment-mapper/test/fixtures-error-handling/node_modules/catch/getmissing.js b/packages/compartment-mapper/test/fixtures-error-handling/node_modules/catch/getmissing.js
diff --git a/packages/compartment-mapper/test/fixtures-error-handling/node_modules/catch/main.js b/packages/compartment-mapper/test/fixtures-error-handling/node_modules/catch/main.js
diff --git a/packages/compartment-mapper/test/fixtures-error-handling/node_modules/catch/package.json b/packages/compartment-mapper/test/fixtures-error-handling/node_modules/catch/package.json
diff --git a/...tment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/README.md b/...tment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/README.md
diff --git a/packages/compartment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/main.js b/packages/compartment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/main.js
diff --git a/.../compartment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/package.json b/.../compartment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/package.json
diff --git a/...mpartment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/throws-falsy.js b/...mpartment-mapper/test/fixtures-error-handling/node_modules/what-the-falsy/throws-falsy.js
diff --git a/packages/compartment-mapper/test/test-error-handling.js b/packages/compartment-mapper/test/test-error-handling.js
@@ -67,3 +67,31 @@ scaffold(
     shouldFailBeforeArchiveOperations: true,
   },
 );
+
+scaffold(
+  'fixtures-error-handling / catch',
+  test,
+  fixtureUrl('fixtures-error-handling/node_modules/catch/main.js'),
+  assertFixture,
+  fixtureAssertionCount,
+  {
+    onError: (t, { error, _title }) => {
+      t.regex(error.message, /obviouslymissing/);
+    },
+    shouldFailBeforeArchiveOperations: false,
+  },
+);
+
+scaffold(
+  'fixtures-error-handling / throw falsy',
+  test,
+  fixtureUrl('fixtures-error-handling/node_modules/what-the-falsy/main.js'),
+  assertFixture,
+  fixtureAssertionCount,
+  {
+    onError: (t, { error, _title }) => {
+      t.assert(!error);
+    },
+    shouldFailBeforeArchiveOperations: false,
+  },
+);
diff --git a/packages/ses/src/module-instance.js b/packages/ses/src/module-instance.js
@@ -17,6 +17,7 @@ import {
   keys,
   mapGet,
   weakmapGet,
+  reflectHas,
 } from './commons.js';
 import { compartmentEvaluate } from './compartment-evaluate.js';
 
@@ -79,20 +80,30 @@ export const makeThirdPartyModuleInstance = (
     };
   }
 
-  let activated = false;
+  const localState = {
+    activated: false,
+  };
   return freeze({
     notifiers,
     exportsProxy,
     execute() {
-      if (!activated) {
+      if (reflectHas(localState, 'errorFromExecute')) {
+        throw localState.errorFromExecute;
+      }
+      if (!localState.activated) {
         activate();
-        activated = true;
-        // eslint-disable-next-line @endo/no-polymorphic-call
-        staticModuleRecord.execute(
-          proxiedExports,
-          compartment,
-          resolvedImports,
-        );
+        localState.activated = true;
+        try {
+          // eslint-disable-next-line @endo/no-polymorphic-call
+          staticModuleRecord.execute(
+            proxiedExports,
+            compartment,
+            resolvedImports,
+          );
+        } catch (err) {
+          localState.errorFromExecute = err;
+          throw err;
+        }
       }
     },
   });