Skip to content

Commit

Permalink
feat(ses): Carry compartment names in error messages (#441)
Browse files Browse the repository at this point in the history
  • Loading branch information
kriskowal committed Aug 26, 2020
1 parent c81ad12 commit 765172a
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 9 deletions.
3 changes: 2 additions & 1 deletion packages/endo/src/assemble.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ export const assemble = (
const compartment = new Compartment(endowments, exitModules, {
resolveHook,
importHook,
moduleMapHook
moduleMapHook,
name: location
});

compartments[compartmentName] = compartment;
Expand Down
7 changes: 6 additions & 1 deletion packages/ses/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ User-visible changes in SES:

## Next release

* No changes yet.
* Adds the `name` option to the `Compartment` constructor and `name` accessor
to the `Compartment` prototype.
Errors that propagate through the module loader will be rethrown anew with
the name of the module and compartment so they can be traced.
At this time, the intermediate stacks of the causal chain are lost.
https://github.com/Agoric/SES-shim/issues/440

## Release 0.10.2 (20-August-2020)

Expand Down
2 changes: 2 additions & 0 deletions packages/ses/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ The `importHook` accepts a full specifier and asynchronously returns a
import 'ses';

const c1 = new Compartment({}, {}, {
name: "first compartment",
resolveHook: (moduleSpecifier, moduleReferrer) => {
return resolve(moduleSpecifier, moduleReferrer);
},
Expand All @@ -171,6 +172,7 @@ module map of another Compartment, creating a link.
const c2 = new Compartment({}, {
'c1': c1.module('./main.js'),
}, {
name: "second compartment",
resolveHook,
importHook,
});
Expand Down
9 changes: 8 additions & 1 deletion packages/ses/src/compartment-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,15 @@ const assertModuleHooks = compartment => {

const CompartmentPrototype = {
constructor: InertCompartment,

get globalThis() {
return privateFields.get(this).globalObject;
},

get name() {
return privateFields.get(this).name;
},

/**
* @param {string} source is a JavaScript program grammar construction.
* @param {{
Expand Down Expand Up @@ -204,6 +209,7 @@ export const makeCompartmentConstructor = (intrinsics, nativeBrander) => {
function Compartment(endowments = {}, moduleMap = {}, options = {}) {
// Extract options, and shallow-clone transforms.
const {
name = '<unknown>',
transforms = [],
globalLexicals = {},
resolveHook,
Expand Down Expand Up @@ -253,7 +259,7 @@ export const makeCompartmentConstructor = (intrinsics, nativeBrander) => {
}

const invalidNames = getOwnPropertyNames(globalLexicals).filter(
name => !isValidIdentifierName(name),
identifier => !isValidIdentifierName(identifier),
);
if (invalidNames.length) {
throw new Error(
Expand All @@ -264,6 +270,7 @@ export const makeCompartmentConstructor = (intrinsics, nativeBrander) => {
}

privateFields.set(this, {
name,
resolveHook,
importHook,
moduleMap,
Expand Down
40 changes: 34 additions & 6 deletions packages/ses/src/module-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,7 @@ const resolveAll = (imports, resolveHook, fullReferrerSpecifier) => {
return freeze(resolvedImports);
};

// `load` asynchronously loads `StaticModuleRecords` and creates a complete
// graph of `ModuleCompartmentRecords`.
// The module records refer to each other by a reference to the dependency's
// compartment and the specifier of the module within its own compartment.
// This graph is then ready to be synchronously linked and executed.
export const load = async (
const loadWithoutErrorAnnotation = async (
compartmentPrivateFields,
moduleAliases,
compartment,
Expand Down Expand Up @@ -73,6 +68,8 @@ export const load = async (
)} because the key is not a module exports namespace, or is from another realm`,
);
}
// Behold: recursion.
// eslint-disable-next-line no-use-before-define
const moduleRecord = await load(
compartmentPrivateFields,
moduleAliases,
Expand Down Expand Up @@ -109,9 +106,40 @@ export const load = async (
// Await all dependencies to load, recursively.
await Promise.all(
values(resolvedImports).map(fullSpecifier =>
// Behold: recursion.
// eslint-disable-next-line no-use-before-define
load(compartmentPrivateFields, moduleAliases, compartment, fullSpecifier),
),
);

return moduleRecord;
};

// `load` asynchronously loads `StaticModuleRecords` and creates a complete
// graph of `ModuleCompartmentRecords`.
// The module records refer to each other by a reference to the dependency's
// compartment and the specifier of the module within its own compartment.
// This graph is then ready to be synchronously linked and executed.
export const load = async (
compartmentPrivateFields,
moduleAliases,
compartment,
moduleSpecifier,
) => {
return loadWithoutErrorAnnotation(
compartmentPrivateFields,
moduleAliases,
compartment,
moduleSpecifier,
).catch(error => {
const { name } = compartmentPrivateFields.get(compartment);
// TODO The following drops the causes' stacks.
// In the future, we should capture the causal chain.
// https://github.com/Agoric/SES-shim/issues/440
throw new error.constructor(
`${error.message}, loading ${q(moduleSpecifier)} in compartment ${q(
name,
)}`,
);
});
};
1 change: 1 addition & 0 deletions packages/ses/src/whitelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,7 @@ export const whitelist = {
constructor: '%InertCompartment%',
evaluate: fn,
globalThis: getter,
name: getter,
import: asyncFn,
load: asyncFn,
importNow: fn,
Expand Down
1 change: 1 addition & 0 deletions packages/ses/test/compartment-instance.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ test('Compartment instance', t => {
'importNow',
'load',
'module',
'name',
'globalThis',
'toString',
].sort(),
Expand Down
1 change: 1 addition & 0 deletions packages/ses/test/compartment-prototype.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ test('Compartment prototype', t => {
'importNow',
'load',
'module',
'name',
'globalThis',
'toString',
].sort(),
Expand Down

0 comments on commit 765172a

Please sign in to comment.