feat(compartment-mapper): defer import errors based on parser support…

… declaration

packages/cjs-module-analyzer/index.js

@@ -1022,7 +1022,6 @@ function tryParseRequire(requireType) {
   // require('...')
   const revertPos = pos;
   if (source.startsWith('equire', pos + 1)) {
-    process._rawDebug("SRC"+source.substr(pos,20))
     pos += 7;
     let ch = commentWhitespace();
     if (ch === 40 /* ( */) {

packages/compartment-mapper/src/archive.js

@@ -5,7 +5,7 @@
 /** @typedef {import('./types.js').ArchiveWriter} ArchiveWriter */
 /** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */
 /** @typedef {import('./types.js').ModuleDescriptor} ModuleDescriptor */
-/** @typedef {import('./types.js').ParseFn} ParseFn */
+/** @typedef {import('./types.js').ParserImplementation} ParserImplementation */
 /** @typedef {import('./types.js').ReadFn} ReadFn */
 /** @typedef {import('./types.js').CaptureSourceLocationHook} CaptureSourceLocationHook */
 /** @typedef {import('./types.js').ReadPowers} ReadPowers */
@@ -19,20 +19,22 @@ import { compartmentMapForNodeModules } from './node-modules.js';
 import { search } from './search.js';
 import { link } from './link.js';
 import { makeImportHookMaker } from './import-hook.js';
-import { parseJson } from './parse-json.js';
-import { parseArchiveCjs } from './parse-archive-cjs.js';
-import { parseArchiveMjs } from './parse-archive-mjs.js';
+import parserJson from './parse-json.js';
+import parserArchiveCjs from './parse-archive-cjs.js';
+import parserArchiveMjs from './parse-archive-mjs.js';
 import { parseLocatedJson } from './json.js';
 import { unpackReadPowers } from './powers.js';
 import { assertCompartmentMap } from './compartment-map.js';
 
 const textEncoder = new TextEncoder();
 
-/** @type {Record<string, ParseFn>} */
+/** @type {Record<string, ParserImplementation>} */
 const parserForLanguage = {
-  mjs: parseArchiveMjs,
-  cjs: parseArchiveCjs,
-  json: parseJson,
+  mjs: parserArchiveMjs,
+  'pre-mjs-json': parserArchiveMjs,
+  cjs: parserArchiveCjs,
+  'pre-cjs-json': parserArchiveCjs,
+  json: parserJson,
 };
 
 /**
@@ -94,7 +96,7 @@ const translateCompartmentMap = (compartments, sources, renames) => {
     if (compartmentSources) {
       for (const name of keys(compartmentSources).sort()) {
         const source = compartmentSources[name];
-        const { location, parser, exit, sha512 } = source;
+        const { location, parser, exit, sha512, deferredError } = source;
         if (location !== undefined) {
           modules[name] = {
             location,
@@ -105,6 +107,10 @@ const translateCompartmentMap = (compartments, sources, renames) => {
           modules[name] = {
             exit,
           };
+        } else if (deferredError !== undefined) {
+          modules[name] = {
+            deferredError,
+          };
         }
       }
     }

packages/compartment-mapper/src/bundle.js

@@ -3,7 +3,7 @@
 
 /** @typedef {import('ses').ResolveHook} ResolveHook */
 /** @typedef {import('ses').PrecompiledStaticModuleInterface} PrecompiledStaticModuleInterface */
-/** @typedef {import('./types.js').ParseFn} ParseFn */
+/** @typedef {import('./types.js').ParserImplementation} ParserImplementation */
 /** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */
 /** @typedef {import('./types.js').CompartmentSources} CompartmentSources */
 /** @typedef {import('./types.js').ReadFn} ReadFn */
@@ -17,21 +17,23 @@ import { compartmentMapForNodeModules } from './node-modules.js';
 import { search } from './search.js';
 import { link } from './link.js';
 import { makeImportHookMaker } from './import-hook.js';
-import { parseJson } from './parse-json.js';
-import { parseArchiveCjs } from './parse-archive-cjs.js';
-import { parseArchiveMjs } from './parse-archive-mjs.js';
+import parserJson from './parse-json.js';
+import parserArchiveCjs from './parse-archive-cjs.js';
+import parserArchiveMjs from './parse-archive-mjs.js';
 import { parseLocatedJson } from './json.js';
 
 const textEncoder = new TextEncoder();
 
 /** quotes strings */
 const q = JSON.stringify;
 
-/** @type {Record<string, ParseFn>} */
+/** @type {Record<string, ParserImplementation>} */
 const parserForLanguage = {
-  mjs: parseArchiveMjs,
-  cjs: parseArchiveCjs,
-  json: parseJson,
+  mjs: parserArchiveMjs,
+  'pre-mjs-json': parserArchiveMjs,
+  cjs: parserArchiveCjs,
+  'pre-cjs-json': parserArchiveCjs,
+  json: parserJson,
 };
 
 /**

packages/compartment-mapper/src/compartment-map.js

@@ -140,13 +140,26 @@ const assertModule = (allegedModule, path, url) => {
     `${path} must be an object, got ${allegedModule} in ${q(url)}`,
   );
 
-  const { compartment, module, location, parser, exit } = moduleDescriptor;
+  const {
+    compartment,
+    module,
+    location,
+    parser,
+    exit,
+    deferredError,
+  } = moduleDescriptor;
   if (compartment !== undefined || module !== undefined) {
     assertCompartmentModule(moduleDescriptor, path, url);
   } else if (location !== undefined || parser !== undefined) {
     assertFileModule(moduleDescriptor, path, url);
   } else if (exit !== undefined) {
     assertExitModule(moduleDescriptor, path, url);
+  } else if (deferredError !== undefined) {
+    assert.typeof(
+      deferredError,
+      'string',
+      `${path}.deferredError must be a string contaiing an error message`,
+    );
   } else {
     assert.fail(
       `${path} is not a valid module descriptor, got ${q(allegedModule)} in ${q(

packages/compartment-mapper/src/import-archive.js

@@ -2,23 +2,24 @@
 /* eslint no-shadow: "off" */
 
 /** @typedef {import('ses').ImportHook} ImportHook */
-/** @typedef {import('./types.js').ParseFn} ParseFn */
+/** @typedef {import('./types.js').ParserImplementation} ParserImplementation */
 /** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */
 /** @typedef {import('./types.js').Application} Application */
 /** @typedef {import('./types.js').CompartmentMapDescriptor} CompartmentMapDescriptor */
 /** @typedef {import('./types.js').ExecuteFn} ExecuteFn */
 /** @typedef {import('./types.js').ReadFn} ReadFn */
 /** @typedef {import('./types.js').ReadPowers} ReadPowers */
 /** @typedef {import('./types.js').HashFn} HashFn */
+/** @typedef {import('./types.js').StaticModuleType} StaticModuleType */
 /** @typedef {import('./types.js').ComputeSourceLocationHook} ComputeSourceLocationHook */
 /** @typedef {import('./types.js').LoadArchiveOptions} LoadArchiveOptions */
 /** @typedef {import('./types.js').ExecuteOptions} ExecuteOptions */
 
 import { ZipReader } from '@endo/zip';
 import { link } from './link.js';
-import { parsePreCjs } from './parse-pre-cjs.js';
-import { parseJson } from './parse-json.js';
-import { parsePreMjs } from './parse-pre-mjs.js';
+import parserPreCjs from './parse-pre-cjs.js';
+import parserJson from './parse-json.js';
+import parserPreMjs from './parse-pre-mjs.js';
 import { parseLocatedJson } from './json.js';
 import { unpackReadPowers } from './powers.js';
 import { join } from './node-module-specifier.js';
@@ -30,11 +31,34 @@ const { quote: q, details: d } = assert;
 
 const textDecoder = new TextDecoder();
 
-/** @type {Record<string, ParseFn>} */
+const { freeze } = Object;
+
+/** @type {Record<string, ParserImplementation>} */
 const parserForLanguage = {
-  'pre-cjs-json': parsePreCjs,
-  'pre-mjs-json': parsePreMjs,
-  json: parseJson,
+  'pre-cjs-json': parserPreCjs,
+  'pre-mjs-json': parserPreMjs,
+  json: parserJson,
+};
+
+/**
+ * @param {string} errorMessage - error to throw on execute
+ * @returns {StaticModuleType}
+ */
+const postponeErrorToExecute = errorMessage => {
+  // Return a place-holder that'd throw an error if executed
+  // This allows cjs parser to more eagerly find calls to require
+  // - if parser identified a require call that's a local function, execute will never be called
+  // - if actual required module is missing, the error will happen anyway - at execution time
+
+  const record = freeze({
+    imports: [],
+    exports: [],
+    execute: () => {
+      throw Error(errorMessage);
+    },
+  });
+
+  return record;
 };
 
 /**
@@ -68,21 +92,24 @@ const makeArchiveImportHookMaker = (
     const importHook = async moduleSpecifier => {
       // per-module:
       const module = modules[moduleSpecifier];
+      if (module.deferredError !== undefined) {
+        return postponeErrorToExecute(module.deferredError);
+      }
       if (module.parser === undefined) {
         throw new Error(
           `Cannot parse module ${q(moduleSpecifier)} in package ${q(
             packageLocation,
           )} in archive ${q(archiveLocation)}`,
         );
       }
-      const parse = parserForLanguage[module.parser];
-      if (parse === undefined) {
+      if (parserForLanguage[module.parser] === undefined) {
         throw new Error(
           `Cannot parse ${q(module.parser)} module ${q(
             moduleSpecifier,
           )} in package ${q(packageLocation)} in archive ${q(archiveLocation)}`,
         );
       }
+      const { parse } = parserForLanguage[module.parser];
       const moduleLocation = `${packageLocation}/${module.location}`;
       const moduleBytes = get(moduleLocation);
 

packages/compartment-mapper/src/import-hook.js

@@ -5,6 +5,7 @@
 /** @typedef {import('./types.js').ReadFn} ReadFn */
 /** @typedef {import('./types.js').HashFn} HashFn */
 /** @typedef {import('./types.js').Sources} Sources */
+/** @typedef {import('./types.js').CompartmentSources} CompartmentSources */
 /** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */
 /** @typedef {import('./types.js').ImportHookMaker} ImportHookMaker */
 
@@ -37,6 +38,11 @@ const has = (haystack, needle) => apply(hasOwnProperty, haystack, [needle]);
  */
 const resolveLocation = (rel, abs) => new URL(rel, abs).toString();
 
+// this is annoying
+function getImportsFromRecord(record) {
+  return (has(record, 'record') ? record.record.imports : record.imports) || [];
+}
+
 /**
  * @param {ReadFn} read
  * @param {string} baseLocation
@@ -54,15 +60,54 @@ export const makeImportHookMaker = (
   exitModules = {},
   computeSha512 = undefined,
 ) => {
+  // Set of specifiers for modules whose parser is not using heuristics to determine imports
+  const strictlyRequired = new Set();
   // per-assembly:
   /** @type {ImportHookMaker} */
-  const makeImportHook = (packageLocation, _packageName, parse) => {
+  const makeImportHook = (
+    packageLocation,
+    _packageName,
+    parse,
+    shouldDeferError,
+  ) => {
     // per-compartment:
     packageLocation = resolveLocation(packageLocation, baseLocation);
     const packageSources = sources[packageLocation] || {};
     sources[packageLocation] = packageSources;
     const { modules = {} } = compartments[packageLocation] || {};
 
+    /**
+     * @param {string} specifier
+     * @param {Error} error - error to throw on execute
+     * @returns {StaticModuleType}
+     */
+    const deferError = (specifier, error) => {
+      // strictlyRequired is populated with imports declared by modules whose parser is not using heuristics to figure
+      // out imports. We're guaranteed they're reachable. If the same module is imported and required, it will not
+      // defer, because importing from esm makes it strictly required.
+      // Note that ultimately a situation may arise, with exit modules, where the module never reaches importHook but
+      // its imports do. In that case the notion of strictly required is no longer boolean, it's true,false,noidea.
+      if (strictlyRequired.has(specifier)) {
+        throw error;
+      }
+      // Return a place-holder that'd throw an error if executed
+      // This allows cjs parser to more eagerly find calls to require
+      // - if parser identified a require call that's a local function, execute will never be called
+      // - if actual required module is missing, the error will happen anyway - at execution time
+      const record = freeze({
+        imports: [],
+        exports: [],
+        execute: () => {
+          throw error;
+        },
+      });
+      packageSources[specifier] = {
+        deferredError: error.message,
+      };
+
+      return record;
+    };
+
     /** @type {ImportHook} */
     const importHook = async moduleSpecifier => {
       // per-module:
@@ -79,10 +124,13 @@ export const makeImportHookMaker = (
           // Archived compartments are not executed.
           return freeze({ imports: [], exports: [], execute() {} });
         }
-        throw new Error(
-          `Cannot find external module ${q(
-            moduleSpecifier,
-          )} in package ${packageLocation}`,
+        return deferError(
+          moduleSpecifier,
+          new Error(
+            `Cannot find external module ${q(
+              moduleSpecifier,
+            )} in package ${packageLocation}`,
+          ),
         );
       }
 
@@ -161,17 +209,27 @@ export const makeImportHookMaker = (
             record,
             sha512,
           };
+          if (!shouldDeferError(parser)) {
+            getImportsFromRecord(record).forEach(
+              strictlyRequired.add,
+              strictlyRequired,
+            );
+          }
+
           return record;
         }
       }
 
-      // TODO offer breadcrumbs in the error message, or how to construct breadcrumbs with another tool.
-      throw new Error(
-        `Cannot find file for internal module ${q(
-          moduleSpecifier,
-        )} (with candidates ${candidates
-          .map(x => q(x))
-          .join(', ')}) in package ${packageLocation}`,
+      return deferError(
+        moduleSpecifier,
+        // TODO offer breadcrumbs in the error message, or how to construct breadcrumbs with another tool.
+        new Error(
+          `Cannot find file for internal module ${q(
+            moduleSpecifier,
+          )} (with candidates ${candidates
+            .map(x => q(x))
+            .join(', ')}) in package ${packageLocation}`,
+        ),
       );
     };
     return importHook;

packages/compartment-mapper/src/import.js

@@ -5,25 +5,25 @@
 /** @typedef {import('./types.js').ArchiveOptions} ArchiveOptions */
 /** @typedef {import('./types.js').ExecuteFn} ExecuteFn */
 /** @typedef {import('./types.js').ExecuteOptions} ExecuteOptions */
-/** @typedef {import('./types.js').ParseFn} ParseFn */
+/** @typedef {import('./types.js').ParserImplementation} ParserImplementation */
 /** @typedef {import('./types.js').ReadFn} ReadFn */
 /** @typedef {import('./types.js').ReadPowers} ReadPowers */
 
 import { compartmentMapForNodeModules } from './node-modules.js';
 import { search } from './search.js';
 import { link } from './link.js';
 import { makeImportHookMaker } from './import-hook.js';
-import { parseJson } from './parse-json.js';
-import { parseCjs } from './parse-cjs.js';
-import { parseMjs } from './parse-mjs.js';
+import parserJson from './parse-json.js';
+import parserCjs from './parse-cjs.js';
+import parserMjs from './parse-mjs.js';
 import { parseLocatedJson } from './json.js';
 import { unpackReadPowers } from './powers.js';
 
-/** @type {Record<string, ParseFn>} */
+/** @type {Record<string, ParserImplementation>} */
 export const parserForLanguage = {
-  mjs: parseMjs,
-  cjs: parseCjs,
-  json: parseJson,
+  mjs: parserMjs,
+  cjs: parserCjs,
+  json: parserJson,
 };
 
 /**