Skip to content

Commit

Permalink
fix: fail template (#1334)
Browse files Browse the repository at this point in the history
  • Loading branch information
erights authored Nov 8, 2022
1 parent a025103 commit 725b987
Show file tree
Hide file tree
Showing 25 changed files with 194 additions and 206 deletions.
10 changes: 3 additions & 7 deletions packages/captp/src/atomics.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="ses"/>

const { details: X } = assert;
const { details: X, Fail } = assert;

// This is a pathological minimum, but exercised by the unit test.
export const MIN_DATA_BUFFER_LENGTH = 1;
Expand All @@ -23,9 +23,7 @@ const STATUS_FLAG_REJECT = 4;
*/
const splitTransferBuffer = transferBuffer => {
transferBuffer.byteLength >= MIN_TRANSFER_BUFFER_LENGTH ||
assert.fail(
X`Transfer buffer of ${transferBuffer.byteLength} bytes is smaller than MIN_TRANSFER_BUFFER_LENGTH ${MIN_TRANSFER_BUFFER_LENGTH}`,
);
Fail`Transfer buffer of ${transferBuffer.byteLength} bytes is smaller than MIN_TRANSFER_BUFFER_LENGTH ${MIN_TRANSFER_BUFFER_LENGTH}`;
const lenbuf = new BigUint64Array(transferBuffer, 0, 1);

// The documentation says that this needs to be an Int32Array for use with
Expand All @@ -40,9 +38,7 @@ const splitTransferBuffer = transferBuffer => {
);
const databuf = new Uint8Array(transferBuffer, overheadLength);
databuf.byteLength >= MIN_DATA_BUFFER_LENGTH ||
assert.fail(
X`Transfer buffer of size ${transferBuffer.byteLength} only supports ${databuf.byteLength} data bytes; need at least ${MIN_DATA_BUFFER_LENGTH}`,
);
Fail`Transfer buffer of size ${transferBuffer.byteLength} only supports ${databuf.byteLength} data bytes; need at least ${MIN_DATA_BUFFER_LENGTH}`;
return harden({ statusbuf, lenbuf, databuf });
};

Expand Down
30 changes: 11 additions & 19 deletions packages/captp/src/captp.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import './types.js';

export { E };

const { details: X } = assert;
const { details: X, Fail } = assert;

/**
* @param {any} maybeThenable
Expand Down Expand Up @@ -61,9 +61,7 @@ export const makeCapTP = (
// more general networks of CapTPs, but we are conservative for at least this
// one case.
!(trapHost && trapGuest) ||
assert.fail(
X`CapTP ${ourId} can only be one of either trapGuest or trapHost`,
);
Fail`CapTP ${ourId} can only be one of either trapGuest or trapHost`;

const disconnectReason = id =>
Error(`${JSON.stringify(id)} connection closed`);
Expand Down Expand Up @@ -390,9 +388,7 @@ export const makeCapTP = (
};
if (trap) {
exportedTrapHandlers.has(val) ||
assert.fail(
X`Refused Trap(${val}) because target was not registered with makeTrapHandler`,
);
Fail`Refused Trap(${val}) because target was not registered with makeTrapHandler`;
assert.typeof(
trapHost,
'function',
Expand Down Expand Up @@ -478,9 +474,7 @@ export const makeCapTP = (
} catch (e) {
cleanup();
if (!e) {
assert.fail(
X`trapGuest expected trapHost AsyncIterator(${questionID}) to be done, but it wasn't`,
);
Fail`trapGuest expected trapHost AsyncIterator(${questionID}) to be done, but it wasn't`;
}
assert.note(e, X`trapHost AsyncIterator(${questionID}) threw`);
throw e;
Expand Down Expand Up @@ -606,18 +600,17 @@ export const makeCapTP = (
// Create the Trap proxy maker.
const makeTrapImpl = implMethod => (target, ...implArgs) => {
Promise.resolve(target) !== target ||
assert.fail(X`Trap(${target}) target cannot be a promise`);
Fail`Trap(${target}) target cannot be a promise`;

const slot = valToSlot.get(target);
// TypeScript confused about `||` control flow so use `if` instead
// https://github.com/microsoft/TypeScript/issues/50739
if (!(slot && slot[1] === '-')) {
assert.fail(X`Trap(${target}) target was not imported`);
Fail`Trap(${target}) target was not imported`;
}
// @ts-expect-error TypeScript confused by `Fail` too?
slot[0] === 't' ||
assert.fail(
X`Trap(${target}) imported target was not created with makeTrapHandler`,
);
Fail`Trap(${target}) imported target was not created with makeTrapHandler`;

// Send a "trap" message.
lastQuestionID += 1;
Expand All @@ -642,14 +635,15 @@ export const makeCapTP = (
break;
}
default: {
assert.fail(X`Internal error; unrecognized implMethod ${implMethod}`);
Fail`Internal error; unrecognized implMethod ${implMethod}`;
}
}

// Set up the trap call with its identifying information and a way to send
// messages over the current CapTP data channel.
const [isException, serialized] = trapGuest({
trapMethod: implMethod,
// @ts-expect-error TypeScript confused by `Fail` too?
slot,
trapArgs: implArgs,
startTrap: () => {
Expand Down Expand Up @@ -683,9 +677,7 @@ export const makeCapTP = (

const value = unserialize(serialized);
!isThenable(value) ||
assert.fail(
X`Trap(${target}) reply cannot be a Thenable; have ${value}`,
);
Fail`Trap(${target}) reply cannot be a Thenable; have ${value}`;

if (isException) {
throw value;
Expand Down
4 changes: 2 additions & 2 deletions packages/captp/test/traplib.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { E, makeCapTP } from '../src/captp.js';

import { makeAtomicsTrapGuest, makeAtomicsTrapHost } from '../src/atomics.js';

const { details: X } = assert;
const { details: X, Fail } = assert;

export const createHostBootstrap = makeTrapHandler => {
// Create a remotable that has a syncable return value.
Expand Down Expand Up @@ -74,7 +74,7 @@ const createGuestBootstrap = (Trap, other) => {
} catch (e) {
return;
}
assert.fail(X`Thunk did not throw: ${ret}`);
Fail`Thunk did not throw: ${ret}`;
},
};
await runTrapTests(mockT, Trap, other, unwrapsPromises);
Expand Down
4 changes: 2 additions & 2 deletions packages/captp/test/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import '@endo/init/debug.js';
import { parentPort } from 'worker_threads';
import { makeGuest, makeHost } from './traplib.js';

const { details: X } = assert;
const { Fail } = assert;

let dispatch;
parentPort.addListener('message', obj => {
switch (obj.type) {
case 'TEST_INIT': {
assert(!dispatch, X`Internal error; duplicate initialization`);
!dispatch || Fail`Internal error; duplicate initialization`;
const { transferBuffer, isGuest } = obj;
const initFn = isGuest ? makeGuest : makeHost;
const ret = initFn(o => parentPort.postMessage(o), transferBuffer);
Expand Down
2 changes: 1 addition & 1 deletion packages/eventual-send/src/track-turns.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// @ts-nocheck

// NOTE: We can't import these because they're not in scope before lockdown.
// import { assert, details as X } from '@agoric/assert';
// import { assert, details as X, Fail } from '@agoric/assert';

// WARNING: Global Mutable State!
// This state is communicated to `assert` that makes it available to the
Expand Down
4 changes: 2 additions & 2 deletions packages/import-bundle/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { parseArchive } from '@endo/compartment-mapper/import-archive.js';
import { decodeBase64 } from '@endo/base64';
import { wrapInescapableCompartment } from './compartment-wrapper.js';

const { details: X } = assert;
const { Fail } = assert;

// importBundle takes the output of bundle-source, and returns a namespace
// object (with .default, and maybe other properties for named exports)
Expand Down Expand Up @@ -82,7 +82,7 @@ export async function importBundle(bundle, options = {}) {
// `filePrefix` and the relative import path of each module.
endowments.nestedEvaluate = src => c.evaluate(src);
} else {
assert.fail(X`unrecognized moduleFormat '${moduleFormat}'`);
Fail`unrecognized moduleFormat '${moduleFormat}'`;
}

c = new CompartmentToUse(endowments, {}, { transforms });
Expand Down
5 changes: 1 addition & 4 deletions packages/import-bundle/test/test-compartment-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ function check(t, c, n) {
t.throws(
() => new Con(),
{
// Temporarily tolerate Endo behavior before and after
// https://github.com/endojs/endo/pull/822
// TODO Simplify once depending on SES post #822
message: /Not available|Function\.prototype\.constructor is not a valid constructor\./,
message: 'Function.prototype.constructor is not a valid constructor.',
},
`${n} .constructor is tamed`,
);
Expand Down
4 changes: 2 additions & 2 deletions packages/marshal/src/dot-membrane.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { passStyleOf } from './passStyleOf.js';

const { fromEntries } = Object;
const { ownKeys } = Reflect;
const { details: X } = assert;
const { Fail } = assert;

// TODO(erights): Add Converter type
/** @param {any} [mirrorConverter] */
Expand Down Expand Up @@ -104,7 +104,7 @@ const makeConverter = (mirrorConverter = undefined) => {
break;
}
default: {
assert.fail(X`internal: Unrecognized passStyle ${passStyle}`);
Fail`internal: Unrecognized passStyle ${passStyle}`;
}
}
mineToYours.set(mine, yours);
Expand Down
64 changes: 25 additions & 39 deletions packages/marshal/src/encodeToCapData.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const {
fromEntries,
freeze,
} = Object;
const { details: X, quote: q } = assert;
const { details: X, Fail, quote: q } = assert;

/**
* Special property name that indicates an encoding that needs special
Expand Down Expand Up @@ -80,14 +80,11 @@ const qclassMatches = (encoded, qclass) =>
* ) => Encoding} [encodeErrorToCapData]
*/

const dontEncodeRemotableToCapData = rem =>
assert.fail(X`remotable unexpected: ${rem}`);
const dontEncodeRemotableToCapData = rem => Fail`remotable unexpected: ${rem}`;

const dontEncodePromiseToCapData = prom =>
assert.fail(X`promise unexpected: ${prom}`);
const dontEncodePromiseToCapData = prom => Fail`promise unexpected: ${prom}`;

const dontEncodeErrorToCapData = err =>
assert.fail(X`error object unexpected: ${err}`);
const dontEncodeErrorToCapData = err => Fail`error object unexpected: ${err}`;

/**
* @param {EncodeToCapDataOptions} encodeOptions
Expand Down Expand Up @@ -221,33 +218,30 @@ export const makeEncodeToCapData = ({
if (qclassMatches(encoded, 'slot')) {
return encoded;
}
assert.fail(
X`internal: Remotable encoding must be an object with ${q(
QCLASS,
)} ${q('slot')}: ${encoded}`,
);
// `throw` is noop since `Fail` throws. But linter confused
throw Fail`internal: Remotable encoding must be an object with ${q(
QCLASS,
)} ${q('slot')}: ${encoded}`;
}
case 'promise': {
const encoded = encodePromiseToCapData(passable, encodeToCapDataRecur);
if (qclassMatches(encoded, 'slot')) {
return encoded;
}
assert.fail(
X`internal: Promise encoding must be an object with ${q(QCLASS)} ${q(
'slot',
)}: ${encoded}`,
);
throw Fail`internal: Promise encoding must be an object with ${q(
QCLASS,
'slot',
)}: ${encoded}`;
}
case 'error': {
const encoded = encodeErrorToCapData(passable, encodeToCapDataRecur);
if (qclassMatches(encoded, 'error')) {
return encoded;
}
assert.fail(
X`internal: Error encoding must be an object with ${q(QCLASS)} ${q(
'error',
)}: ${encoded}`,
);
throw Fail`internal: Error encoding must be an object with ${q(
QCLASS,
'error',
)}: ${encoded}`;
}
default: {
assert.fail(
Expand Down Expand Up @@ -294,9 +288,9 @@ harden(makeEncodeToCapData);
*/

const dontDecodeRemotableOrPromiseFromCapData = slotEncoding =>
assert.fail(X`remotable or promise unexpected: ${slotEncoding}`);
Fail`remotable or promise unexpected: ${slotEncoding}`;
const dontDecodeErrorFromCapData = errorEncoding =>
assert.fail(X`error unexpected: ${errorEncoding}`);
Fail`error unexpected: ${errorEncoding}`;

/**
* The current encoding does not give the decoder enough into to distinguish
Expand Down Expand Up @@ -427,9 +421,7 @@ export const makeDecodeFromCapData = ({
if (passStyleOf(decoded) === 'error') {
return decoded;
}
assert.fail(
X`internal: decodeErrorFromCapData option must return an error: ${decoded}`,
);
throw Fail`internal: decodeErrorFromCapData option must return an error: ${decoded}`;
}
case 'hilbert': {
// Using @ts-ignore rather than @ts-expect-error below because
Expand All @@ -439,7 +431,7 @@ export const makeDecodeFromCapData = ({
// See https://github.com/endojs/endo/pull/1259#discussion_r954561901
const { original, rest } = jsonEncoded;
hasOwnPropertyOf(jsonEncoded, 'original') ||
assert.fail(X`Invalid Hilbert Hotel encoding ${jsonEncoded}`);
Fail`Invalid Hilbert Hotel encoding ${jsonEncoded}`;
// Don't harden since we're not done mutating it
const result = { [QCLASS]: decodeFromCapData(original) };
if (hasOwnPropertyOf(jsonEncoded, 'rest')) {
Expand All @@ -454,20 +446,16 @@ export const makeDecodeFromCapData = ({
// `'copyRecord'` but we'd have to harden it and it is too
// early to do that.
!hasOwnPropertyOf(restObj, QCLASS) ||
assert.fail(
X`Rest must not contain its own definition of ${q(QCLASS)}`,
);
Fail`Rest must not contain its own definition of ${q(QCLASS)}`;
defineProperties(result, getOwnPropertyDescriptors(restObj));
}
return result;
}
// @ts-expect-error This is the error case we're testing for
case 'ibid': {
assert.fail(
X`The capData protocol no longer supports ${q(QCLASS)} ${q(
qclass,
)}`,
);
throw Fail`The capData protocol no longer supports ${q(QCLASS)} ${q(
qclass,
)}`;
}
default: {
assert.fail(X`unrecognized ${q(QCLASS)} ${q(qclass)}`, TypeError);
Expand All @@ -477,9 +465,7 @@ export const makeDecodeFromCapData = ({
assert(typeof jsonEncoded === 'object' && jsonEncoded !== null);
const decodeEntry = ([name, encodedVal]) => {
typeof name === 'string' ||
assert.fail(
X`Property ${q(name)} of ${jsonEncoded} must be a string`,
);
Fail`Property ${q(name)} of ${jsonEncoded} must be a string`;
return [name, decodeFromCapData(encodedVal)];
};
const decodedEntries = entries(jsonEncoded).map(decodeEntry);
Expand Down
Loading

0 comments on commit 725b987

Please sign in to comment.