From 725b987ffa812a070ff45fcd496cf8fd88df6963 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Mon, 7 Nov 2022 17:06:36 -0800 Subject: [PATCH] fix: fail template (#1334) --- packages/captp/src/atomics.js | 10 +-- packages/captp/src/captp.js | 30 +++---- packages/captp/test/traplib.js | 4 +- packages/captp/test/worker.js | 4 +- packages/eventual-send/src/track-turns.js | 2 +- packages/import-bundle/src/index.js | 4 +- .../test/test-compartment-wrapper.js | 5 +- packages/marshal/src/dot-membrane.js | 4 +- packages/marshal/src/encodeToCapData.js | 64 ++++++-------- packages/marshal/src/encodeToSmallcaps.js | 86 +++++++------------ packages/marshal/src/helpers/remotable.js | 4 +- packages/marshal/src/helpers/symbol.js | 8 +- packages/marshal/src/helpers/tagged.js | 6 +- packages/marshal/src/make-far.js | 12 +-- packages/marshal/src/marshal-justin.js | 13 ++- packages/marshal/src/marshal-stringify.js | 9 +- packages/marshal/src/marshal.js | 27 +++--- packages/marshal/src/passStyleOf.js | 16 ++-- packages/ses/index.d.ts | 1 + packages/ses/src/environment-options.js | 10 +-- packages/ses/src/error/assert.js | 4 + packages/ses/src/error/types.js | 59 +++++++++++++ .../test/fixtures/exportheavy.js | 6 +- packages/stream-node/reader.js | 6 +- packages/stream-node/writer.js | 6 +- 25 files changed, 194 insertions(+), 206 deletions(-) diff --git a/packages/captp/src/atomics.js b/packages/captp/src/atomics.js index ed44d1d8c7..6eeeb0e940 100644 --- a/packages/captp/src/atomics.js +++ b/packages/captp/src/atomics.js @@ -1,6 +1,6 @@ /// -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; @@ -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 @@ -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 }); }; diff --git a/packages/captp/src/captp.js b/packages/captp/src/captp.js index 064e5723b6..8da1434fb3 100644 --- a/packages/captp/src/captp.js +++ b/packages/captp/src/captp.js @@ -14,7 +14,7 @@ import './types.js'; export { E }; -const { details: X } = assert; +const { details: X, Fail } = assert; /** * @param {any} maybeThenable @@ -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`); @@ -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', @@ -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; @@ -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; @@ -642,7 +635,7 @@ export const makeCapTP = ( break; } default: { - assert.fail(X`Internal error; unrecognized implMethod ${implMethod}`); + Fail`Internal error; unrecognized implMethod ${implMethod}`; } } @@ -650,6 +643,7 @@ export const makeCapTP = ( // 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: () => { @@ -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; diff --git a/packages/captp/test/traplib.js b/packages/captp/test/traplib.js index 967c6dc384..0c796531f6 100644 --- a/packages/captp/test/traplib.js +++ b/packages/captp/test/traplib.js @@ -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. @@ -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); diff --git a/packages/captp/test/worker.js b/packages/captp/test/worker.js index b3ecbcb271..2dfd477bcc 100644 --- a/packages/captp/test/worker.js +++ b/packages/captp/test/worker.js @@ -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); diff --git a/packages/eventual-send/src/track-turns.js b/packages/eventual-send/src/track-turns.js index abd7630b91..efd07b2dea 100644 --- a/packages/eventual-send/src/track-turns.js +++ b/packages/eventual-send/src/track-turns.js @@ -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 diff --git a/packages/import-bundle/src/index.js b/packages/import-bundle/src/index.js index 51b47153f7..5629178418 100644 --- a/packages/import-bundle/src/index.js +++ b/packages/import-bundle/src/index.js @@ -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) @@ -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 }); diff --git a/packages/import-bundle/test/test-compartment-wrapper.js b/packages/import-bundle/test/test-compartment-wrapper.js index ba0f00a9ad..12c691f9b3 100644 --- a/packages/import-bundle/test/test-compartment-wrapper.js +++ b/packages/import-bundle/test/test-compartment-wrapper.js @@ -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`, ); diff --git a/packages/marshal/src/dot-membrane.js b/packages/marshal/src/dot-membrane.js index 4dd35e65c2..cc4b303ae6 100644 --- a/packages/marshal/src/dot-membrane.js +++ b/packages/marshal/src/dot-membrane.js @@ -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] */ @@ -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); diff --git a/packages/marshal/src/encodeToCapData.js b/packages/marshal/src/encodeToCapData.js index bb5b1ca54e..0099c105b6 100644 --- a/packages/marshal/src/encodeToCapData.js +++ b/packages/marshal/src/encodeToCapData.js @@ -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 @@ -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 @@ -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( @@ -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 @@ -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 @@ -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')) { @@ -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); @@ -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); diff --git a/packages/marshal/src/encodeToSmallcaps.js b/packages/marshal/src/encodeToSmallcaps.js index 15f94d14eb..27eb95579c 100644 --- a/packages/marshal/src/encodeToSmallcaps.js +++ b/packages/marshal/src/encodeToSmallcaps.js @@ -29,7 +29,7 @@ import { const { ownKeys } = Reflect; const { isArray } = Array; const { is, entries, fromEntries } = Object; -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; const BANG = '!'.charCodeAt(0); const DASH = '-'.charCodeAt(0); @@ -105,13 +105,12 @@ const startsSpecial = encodedStr => { */ const dontEncodeRemotableToSmallcaps = rem => - assert.fail(X`remotable unexpected: ${rem}`); + Fail`remotable unexpected: ${rem}`; -const dontEncodePromiseToSmallcaps = prom => - assert.fail(X`promise unexpected: ${prom}`); +const dontEncodePromiseToSmallcaps = prom => Fail`promise unexpected: ${prom}`; const dontEncodeErrorToSmallcaps = err => - assert.fail(X`error object unexpected: ${q(err)}`); + Fail`error object unexpected: ${q(err)}`; /** * @param {EncodeToSmallcapsOptions} encodeOptions @@ -238,9 +237,8 @@ export const makeEncodeToSmallcaps = ({ if (typeof result === 'string' && result.startsWith('$')) { return result; } - assert.fail( - X`internal: Remotable encoding must start with "$": ${result}`, - ); + // `throw` is noop since `Fail` throws. But linter confused + throw Fail`internal: Remotable encoding must start with "$": ${result}`; } case 'promise': { const result = encodePromiseToSmallcaps( @@ -250,9 +248,7 @@ export const makeEncodeToSmallcaps = ({ if (typeof result === 'string' && result.startsWith('&')) { return result; } - assert.fail( - X`internal: Promise encoding must start with "&": ${result}`, - ); + throw Fail`internal: Promise encoding must start with "&": ${result}`; } case 'error': { const result = encodeErrorToSmallcaps(passable, encodeToSmallcapsRecur); @@ -265,13 +261,13 @@ export const makeEncodeToSmallcaps = ({ ) { return result; } - assert.fail( - X`internal: Error encoding must have string message: ${q(message)}`, - ); + Fail`internal: Error encoding must have string message: ${q( + message, + )}`; } - assert.fail( - X`internal: Error encoding must have "#error" property: ${q(result)}`, - ); + throw Fail`internal: Error encoding must have "#error" property: ${q( + result, + )}`; } default: { assert.fail( @@ -305,13 +301,9 @@ export const makeEncodeToSmallcaps = ({ ) { return result; } - assert.fail( - X`internal: Error encoding must string message: ${q(message)}`, - ); + Fail`internal: Error encoding must string message: ${q(message)}`; } - assert.fail( - X`internal: Error encoding must have "#error" property: ${q(result)}`, - ); + Fail`internal: Error encoding must have "#error" property: ${q(result)}`; } return harden(encodeToSmallcapsRecur(passable)); }; @@ -336,11 +328,11 @@ harden(makeEncodeToSmallcaps); */ const dontDecodeRemotableFromSmallcaps = encoding => - assert.fail(X`remotable unexpected: ${encoding}`); + Fail`remotable unexpected: ${encoding}`; const dontDecodePromiseFromSmallcaps = encoding => - assert.fail(X`promise unexpected: ${encoding}`); + Fail`promise unexpected: ${encoding}`; const dontDecodeErrorFromSmallcaps = encoding => - assert.fail(X`error unexpected: ${q(encoding)}`); + Fail`error unexpected: ${q(encoding)}`; /** * @param {DecodeFromSmallcapsOptions} decodeOptions @@ -411,9 +403,7 @@ export const makeDecodeFromSmallcaps = ({ decodeFromSmallcaps, ); if (passStyleOf(result) !== 'remotable') { - assert.fail( - X`internal: decodeRemotableFromSmallcaps option must return a remotable: ${result}`, - ); + Fail`internal: decodeRemotableFromSmallcaps option must return a remotable: ${result}`; } return result; } @@ -423,16 +413,14 @@ export const makeDecodeFromSmallcaps = ({ decodeFromSmallcaps, ); if (passStyleOf(result) !== 'promise') { - assert.fail( - X`internal: decodePromiseFromSmallcaps option must return a promise: ${result}`, - ); + Fail`internal: decodePromiseFromSmallcaps option must return a promise: ${result}`; } return result; } default: { - assert.fail( - X`Special char ${q(c)} reserved for future use: ${encoding}`, - ); + throw Fail`Special char ${q( + c, + )} reserved for future use: ${encoding}`; } } } @@ -448,13 +436,9 @@ export const makeDecodeFromSmallcaps = ({ if (hasOwnPropertyOf(encoding, '#tag')) { const { '#tag': tag, payload, ...rest } = encoding; typeof tag === 'string' || - assert.fail( - X`Value of "#tag", the tag, must be a string: ${encoding}`, - ); + Fail`Value of "#tag", the tag, must be a string: ${encoding}`; ownKeys(rest).length === 0 || - assert.fail( - X`#tag record unexpected properties: ${q(ownKeys(rest))}`, - ); + Fail`#tag record unexpected properties: ${q(ownKeys(rest))}`; return makeTagged( decodeFromSmallcaps(tag), decodeFromSmallcaps(payload), @@ -467,28 +451,20 @@ export const makeDecodeFromSmallcaps = ({ decodeFromSmallcaps, ); passStyleOf(result) === 'error' || - assert.fail( - X`internal: decodeErrorFromSmallcaps option must return an error: ${result}`, - ); + Fail`internal: decodeErrorFromSmallcaps option must return an error: ${result}`; return result; } const decodeEntry = ([encodedName, encodedVal]) => { typeof encodedName === 'string' || - assert.fail( - X`Property name ${q( - encodedName, - )} of ${encoding} must be a string`, - ); + Fail`Property name ${q( + encodedName, + )} of ${encoding} must be a string`; !encodedName.startsWith('#') || - assert.fail( - X`Unrecognized record type ${q(encodedName)}: ${encoding}`, - ); + Fail`Unrecognized record type ${q(encodedName)}: ${encoding}`; const name = decodeFromSmallcaps(encodedName); typeof name === 'string' || - assert.fail( - X`Decoded property name ${name} from ${encoding} must be a string`, - ); + Fail`Decoded property name ${name} from ${encoding} must be a string`; return [name, decodeFromSmallcaps(encodedVal)]; }; const decodedEntries = entries(encoding).map(decodeEntry); diff --git a/packages/marshal/src/helpers/remotable.js b/packages/marshal/src/helpers/remotable.js index a69eb31f37..5c1b68d58f 100644 --- a/packages/marshal/src/helpers/remotable.js +++ b/packages/marshal/src/helpers/remotable.js @@ -19,7 +19,7 @@ import { /** @typedef {import('./internal-types.js').PassStyleHelper} PassStyleHelper */ /** @typedef {import('../types.js').Remotable} Remotable */ -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; const { ownKeys } = Reflect; const { isArray } = Array; const { @@ -74,7 +74,7 @@ harden(assertIface); const checkRemotableProtoOf = (original, check) => { const reject = !!check && (details => check(false, details)); isObject(original) || - assert.fail(X`Remotables must be objects or functions: ${original}`); + Fail`Remotables must be objects or functions: ${original}`; // A valid remotable object must inherit from a "tag record" -- a // plain-object prototype consisting of only diff --git a/packages/marshal/src/helpers/symbol.js b/packages/marshal/src/helpers/symbol.js index eb634bc605..c50af94976 100644 --- a/packages/marshal/src/helpers/symbol.js +++ b/packages/marshal/src/helpers/symbol.js @@ -1,6 +1,6 @@ // @ts-check -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; const { ownKeys } = Reflect; /** @@ -40,9 +40,7 @@ harden(isPassableSymbol); export const assertPassableSymbol = sym => isPassableSymbol(sym) || - assert.fail( - X`Only registered symbols or well-known symbols are passable: ${q(sym)}`, - ); + Fail`Only registered symbols or well-known symbols are passable: ${q(sym)}`; harden(assertPassableSymbol); /** @@ -114,7 +112,7 @@ export const passableSymbolForName = name => { if (typeof sym === 'symbol') { return sym; } - assert.fail(X`Reserved for well known symbol ${q(suffix)}: ${q(name)}`); + Fail`Reserved for well known symbol ${q(suffix)}: ${q(name)}`; } } return Symbol.for(name); diff --git a/packages/marshal/src/helpers/tagged.js b/packages/marshal/src/helpers/tagged.js index b2068ac0bd..36907cc6f2 100644 --- a/packages/marshal/src/helpers/tagged.js +++ b/packages/marshal/src/helpers/tagged.js @@ -10,7 +10,7 @@ import { checkPassStyle, } from './passStyle-helpers.js'; -const { details: X } = assert; +const { Fail } = assert; const { ownKeys } = Reflect; const { getOwnPropertyDescriptors } = Object; @@ -37,9 +37,7 @@ export const TaggedHelper = harden({ ...restDescs } = getOwnPropertyDescriptors(candidate); (ownKeys(restDescs).length === 0) || - assert.fail( - X`Unexpected properties on tagged record ${ownKeys(restDescs)}`, - ); + Fail`Unexpected properties on tagged record ${ownKeys(restDescs)}`; checkNormalProperty(candidate, 'payload', true, assertChecker); diff --git a/packages/marshal/src/make-far.js b/packages/marshal/src/make-far.js index 50539e9b85..cf1bbce5ad 100644 --- a/packages/marshal/src/make-far.js +++ b/packages/marshal/src/make-far.js @@ -12,7 +12,7 @@ import { /** @typedef {import('./types.js').InterfaceSpec} InterfaceSpec */ /** @template L,R @typedef {import('@endo/eventual-send').RemotableBrand} RemotableBrand */ -const { quote: q, details: X } = assert; +const { quote: q, details: X, Fail } = assert; const { prototype: functionPrototype } = Function; const { @@ -39,21 +39,17 @@ const makeRemotableProto = (remotable, iface) => { oldProto = objectPrototype; } oldProto === objectPrototype || - assert.fail( - X`For now, remotables cannot inherit from anything unusual, in ${remotable}`, - ); + Fail`For now, remotables cannot inherit from anything unusual, in ${remotable}`; } else if (typeof remotable === 'function') { oldProto !== null || - assert.fail( - X`Original function must not inherit from null: ${remotable}`, - ); + Fail`Original function must not inherit from null: ${remotable}`; assert( oldProto === functionPrototype || getPrototypeOf(oldProto) === functionPrototype, X`Far functions must originally inherit from Function.prototype, in ${remotable}`, ); } else { - assert.fail(X`unrecognized typeof ${remotable}`); + Fail`unrecognized typeof ${remotable}`; } return harden( create(oldProto, { diff --git a/packages/marshal/src/marshal-justin.js b/packages/marshal/src/marshal-justin.js index 635a593c2a..a5ba4adb9d 100644 --- a/packages/marshal/src/marshal-justin.js +++ b/packages/marshal/src/marshal-justin.js @@ -14,7 +14,7 @@ import { AtAtPrefixPattern, passableSymbolForName } from './helpers/symbol.js'; const { ownKeys } = Reflect; const { isArray } = Array; const { stringify: quote } = JSON; -const { quote: q, details: X } = assert; +const { quote: q, details: X, Fail } = assert; /** * @typedef {Object} Indenter @@ -190,7 +190,7 @@ const decodeToJustin = (encoding, shouldIndent = false) => { case 'hilbert': { const { original, rest } = rawTree; 'original' in rawTree || - assert.fail(X`Invalid Hilbert Hotel encoding ${rawTree}`); + Fail`Invalid Hilbert Hotel encoding ${rawTree}`; prepare(original); if ('rest' in rawTree) { assert.typeof( @@ -199,12 +199,9 @@ const decodeToJustin = (encoding, shouldIndent = false) => { X`Rest ${rest} encoding must be an object`, ); assert(rest !== null, X`Rest ${rest} encoding must not be null`); - !isArray(rest) || - assert.fail(X`Rest ${rest} encoding must not be an array`); + !isArray(rest) || Fail`Rest ${rest} encoding must not be an array`; !(QCLASS in rest) || - assert.fail( - X`Rest encoding ${rest} must not contain ${q(QCLASS)}`, - ); + Fail`Rest encoding ${rest} must not contain ${q(QCLASS)}`; const names = ownKeys(rest); for (const name of names) { assert.typeof( @@ -225,7 +222,7 @@ const decodeToJustin = (encoding, shouldIndent = false) => { X`invalid error name typeof ${q(typeof name)}`, ); getErrorConstructor(name) !== undefined || - assert.fail(X`Must be the name of an Error constructor ${name}`); + Fail`Must be the name of an Error constructor ${name}`; assert.typeof( message, 'string', diff --git a/packages/marshal/src/marshal-stringify.js b/packages/marshal/src/marshal-stringify.js index c6a32061a7..1c5558194c 100644 --- a/packages/marshal/src/marshal-stringify.js +++ b/packages/marshal/src/marshal-stringify.js @@ -5,22 +5,23 @@ import { makeMarshal } from './marshal.js'; /** @typedef {import('./types.js').Passable} Passable */ -const { details: X } = assert; +const { Fail } = assert; /** @type {import('./types.js').ConvertValToSlot} */ const doNotConvertValToSlot = val => - assert.fail(X`Marshal's stringify rejects presences and promises ${val}`); + Fail`Marshal's stringify rejects presences and promises ${val}`; /** @type {import('./types.js').ConvertSlotToVal} */ const doNotConvertSlotToVal = (slot, _iface) => - assert.fail(X`Marshal's parse must not encode any slots ${slot}`); + Fail`Marshal's parse must not encode any slots ${slot}`; const badArrayHandler = harden({ get: (_target, name, _receiver) => { if (name === 'length') { return 0; } - assert.fail(X`Marshal's parse must not encode any slot positions ${name}`); + // `throw` is noop since `Fail` throws. But linter confused + throw Fail`Marshal's parse must not encode any slot positions ${name}`; }, }); diff --git a/packages/marshal/src/marshal.js b/packages/marshal/src/marshal.js index 83def9e883..95e1ddd99b 100644 --- a/packages/marshal/src/marshal.js +++ b/packages/marshal/src/marshal.js @@ -29,7 +29,7 @@ import { hasOwnPropertyOf } from './helpers/passStyle-helpers.js'; /** @typedef {import('./types.js').Remotable} Remotable */ const { isArray } = Array; -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; const { ownKeys } = Reflect; /** @type {ConvertValToSlot} */ @@ -64,9 +64,7 @@ export const makeMarshal = ( assert.typeof(marshalName, 'string'); errorTagging === 'on' || errorTagging === 'off' || - assert.fail( - X`The errorTagging option can only be "on" or "off" ${errorTagging}`, - ); + Fail`The errorTagging option can only be "on" or "off" ${errorTagging}`; const nextErrorId = () => { errorIdNum += 1; return `error:${marshalName}#${errorIdNum}`; @@ -227,9 +225,8 @@ export const makeMarshal = ( slots, }); } else { - assert.fail( - X`Unrecognized serializeBodyFormat: ${q(serializeBodyFormat)}`, - ); + // The `throw` is a noop since `Fail` throws. Added for confused linters. + throw Fail`Unrecognized serializeBodyFormat: ${q(serializeBodyFormat)}`; } }; @@ -244,7 +241,7 @@ export const makeMarshal = ( const decodeSlotCommon = slotData => { const { iface = undefined, index, ...rest } = slotData; ownKeys(rest).length === 0 || - assert.fail(X`unexpected encoded slot properties ${q(ownKeys(rest))}`); + Fail`unexpected encoded slot properties ${q(ownKeys(rest))}`; if (valMap.has(index)) { return valMap.get(index); } @@ -264,7 +261,7 @@ export const makeMarshal = ( const decodeErrorCommon = (errData, decodeRecur) => { const { errorId = undefined, message, name, ...rest } = errData; ownKeys(rest).length === 0 || - assert.fail(X`unexpected encoded error properties ${q(ownKeys(rest))}`); + Fail`unexpected encoded error properties ${q(ownKeys(rest))}`; // TODO Must decode `cause` and `errors` properties // capData does not transform strings. The calls to `decodeRecur` // are for reuse by other encodings that do, such as smallcaps. @@ -272,9 +269,9 @@ export const makeMarshal = ( const dMessage = decodeRecur(message); const dErrorId = errorId && decodeRecur(errorId); typeof dName === 'string' || - assert.fail(X`invalid error name typeof ${q(typeof dName)}`); + Fail`invalid error name typeof ${q(typeof dName)}`; typeof dMessage === 'string' || - assert.fail(X`invalid error message typeof ${q(typeof dMessage)}`); + Fail`invalid error message typeof ${q(typeof dMessage)}`; const EC = getErrorConstructor(dName) || Error; // errorId is a late addition so be tolerant of its absence. const errorName = @@ -323,7 +320,7 @@ export const makeMarshal = ( const decodeErrorFromSmallcaps = (encoding, decodeRecur) => { const { '#error': message, ...restErrData } = encoding; !hasOwnPropertyOf(restErrData, 'message') || - assert.fail(X`unexpected encoded error property ${q('message')}`); + Fail`unexpected encoded error property ${q('message')}`; return decodeErrorCommon({ message, ...restErrData }, decodeRecur); }; @@ -342,11 +339,9 @@ export const makeMarshal = ( const unserialize = data => { const { body, slots } = data; typeof body === 'string' || - assert.fail( - X`unserialize() given non-capdata (.body is ${body}, not string)`, - ); + Fail`unserialize() given non-capdata (.body is ${body}, not string)`; isArray(data.slots) || - assert.fail(X`unserialize() given non-capdata (.slots are not Array)`); + Fail`unserialize() given non-capdata (.slots are not Array)`; const { reviveFromCapData, reviveFromSmallcaps } = makeFullRevive(slots); let result; // JSON cannot begin with a '#', so this is an unambiguous signal. diff --git a/packages/marshal/src/passStyleOf.js b/packages/marshal/src/passStyleOf.js index 39b8e5a93d..a164f054c4 100644 --- a/packages/marshal/src/passStyleOf.js +++ b/packages/marshal/src/passStyleOf.js @@ -26,7 +26,7 @@ import { assertSafePromise } from './helpers/safe-promise.js'; /** @typedef {Exclude} HelperPassStyle */ -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; const { ownKeys } = Reflect; const { isFrozen } = Object; @@ -57,7 +57,7 @@ const makeHelperTable = passStyleHelpers => { } for (const styleName of ownKeys(HelperTable)) { HelperTable[styleName] !== undefined || - assert.fail(X`missing helper for ${q(styleName)}`); + Fail`missing helper for ${q(styleName)}`; } return harden(HelperTable); @@ -112,7 +112,7 @@ const makePassStyleOf = passStyleHelpers => { return passStyleMemo.get(inner); } (!inProgress.has(inner)) || - assert.fail(X`Pass-by-copy data cannot be cyclic ${inner}`); + Fail`Pass-by-copy data cannot be cyclic ${inner}`; inProgress.add(inner); } // eslint-disable-next-line no-use-before-define @@ -159,13 +159,13 @@ const makePassStyleOf = passStyleHelpers => { return 'promise'; } (typeof inner.then !== 'function') || - assert.fail(X`Cannot pass non-promise thenables`); + Fail`Cannot pass non-promise thenables`; const passStyleTag = inner[PASS_STYLE]; if (passStyleTag !== undefined) { assert.typeof(passStyleTag, 'string'); const helper = HelperTable[passStyleTag]; (helper !== undefined) || - assert.fail(X`Unrecognized PassStyle: ${q(passStyleTag)}`); + Fail`Unrecognized PassStyle: ${q(passStyleTag)}`; helper.assertValid(inner, passStyleOfRecur); return /** @type {PassStyle} */ (passStyleTag); } @@ -180,11 +180,9 @@ const makePassStyleOf = passStyleHelpers => { } case 'function': { (isFrozen(inner)) || - assert.fail( - X`Cannot pass non-frozen objects like ${inner}. Use harden()`, - ); + Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`; (typeof inner.then !== 'function') || - assert.fail(X`Cannot pass non-promise thenables`); + Fail`Cannot pass non-promise thenables`; remotableHelper.assertValid(inner, passStyleOfRecur); return 'remotable'; } diff --git a/packages/ses/index.d.ts b/packages/ses/index.d.ts index c69a51a84c..1cde596bac 100644 --- a/packages/ses/index.d.ts +++ b/packages/ses/index.d.ts @@ -195,6 +195,7 @@ export interface Assert { template: TemplateStringsArray | string[], ...args: any ): DetailsToken; + Fail(template: TemplateStringsArray | string[], ...args: any): never; quote(payload: any, spaces?: string | number): ToStringable; makeAssert: MakeAssert; } diff --git a/packages/ses/src/environment-options.js b/packages/ses/src/environment-options.js index 0a61145785..e0e0965a51 100644 --- a/packages/ses/src/environment-options.js +++ b/packages/ses/src/environment-options.js @@ -5,7 +5,7 @@ import { arrayPush, freeze } from './commons.js'; import { assert } from './error/assert.js'; -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; /** * JavaScript module semantics resists attempts to parameterize a module's @@ -110,11 +110,9 @@ export const makeEnvironmentCaptor = aGlobal => { setting === undefined || typeof setting === 'string' || // eslint-disable-next-line @endo/no-polymorphic-call - assert.fail( - X`Environment option value ${q( - setting, - )}, if present, must be a string.`, - ); + Fail`Environment option value ${q( + setting, + )}, if present, must be a string.`; return setting; }; freeze(getEnvironmentOption); diff --git a/packages/ses/src/error/assert.js b/packages/ses/src/error/assert.js index a21102c97e..db7ecddd98 100644 --- a/packages/ses/src/error/assert.js +++ b/packages/ses/src/error/assert.js @@ -375,6 +375,9 @@ const makeAssert = (optRaise = undefined, unredacted = false) => { }; freeze(fail); + /** @type {FailTag} */ + const Fail = (template, ...args) => fail(details(template, ...args)); + // Don't freeze or export `baseAssert` until we add methods. // TODO If I change this from a `function` function to an arrow // function, I seem to get type errors from TypeScript. Why? @@ -433,6 +436,7 @@ const makeAssert = (optRaise = undefined, unredacted = false) => { string: assertString, note, details, + Fail, quote, makeAssert, }); diff --git a/packages/ses/src/error/types.js b/packages/ses/src/error/types.js index 75d18f7112..3b6f28cc94 100644 --- a/packages/ses/src/error/types.js +++ b/packages/ses/src/error/types.js @@ -260,6 +260,64 @@ * `string[]` can also be used as a template. */ +/** + * @typedef {(template: TemplateStringsArray | string[], ...args: any) => never} FailTag + * The `Fail` tamplate tag supports replacing patterns like + * ```js + * assert(cond, X`...complaint...`); + * ``` + * or + * ```js + * cond || assert.fail(X`...complaint...`); + * ``` + * with patterns like + * ```js + * cond || Fail`...complaint...`; + * ``` + * + * However, due to [weakness in current + * TypeScript](https://github.com/microsoft/TypeScript/issues/51426), the `||` + * patterns are not as powerful as the `assert(...)` call at enabling static + * reasoning. Of the `||`, again due to weaknesses in current TypeScript, + * the + * ```js + * cond || Fail`...complaint...` + * ``` + * pattern is not as powerful as the + * ```js + * cond || assert.fail(X`...complaint...`); + * ``` + * at enabling static resoning. Despite these problems, we do not want to + * return to the + * ```js + * assert(cond, X`...complaint...`) + * ``` + * style because of the substantial overhead in + * evaluating the `X` template in the typical `true` case where it is not + * needed. And we do not want to return to the + * ```js + * assert.fail(X`...complaint...`)` + * ``` + * because of the verbosity and loss of readability. Instead, until/unless + * https://github.com/microsoft/TypeScript/issues/51426 is fixed, for those + * new-style assertions where this loss of static reasoning is a problem, + * instead express the assertion as + * ```js + * if (!cond) { + * Fail`...complaint...`; + * } + * ``` + * or, if needed, + * ```js + * if (!cond) { + * // `throw` is noop since `Fail` throws. But linter confused + * throw Fail`...complaint...`; + * } + * ``` + * This avoid the TypeScript bugs that cause the loss of static reasoning, + * but with no loss of efficiency and little loss of readability. + */ + /** * assert that expr is truthy, with an optional details to describe * the assertion. It is a tagged template literal like @@ -286,6 +344,7 @@ * string: AssertString, * note: AssertNote, * details: DetailsTag, + * Fail: FailTag, * quote: AssertQuote, * makeAssert: MakeAssert, * } } Assert diff --git a/packages/static-module-record/test/fixtures/exportheavy.js b/packages/static-module-record/test/fixtures/exportheavy.js index fc65ec49a9..ef14e708ee 100644 --- a/packages/static-module-record/test/fixtures/exportheavy.js +++ b/packages/static-module-record/test/fixtures/exportheavy.js @@ -1,4 +1,4 @@ -/* eslint-disable */ +/* eslint-disable */ console.error("This is a code sample for trying out babel transforms, it's not meant to be run"); export { mapIterable, filterIterable } from './src/helpers/iter-helpers.js'; @@ -43,7 +43,7 @@ export { export * from './src/types.js'; -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; @@ -52,4 +52,4 @@ export const MIN_DATA_BUFFER_LENGTH = 1; export const TRANSFER_OVERHEAD_LENGTH = BigUint64Array.BYTES_PER_ELEMENT + Int32Array.BYTES_PER_ELEMENT; export const MIN_TRANSFER_BUFFER_LENGTH = - MIN_DATA_BUFFER_LENGTH + TRANSFER_OVERHEAD_LENGTH; \ No newline at end of file + MIN_DATA_BUFFER_LENGTH + TRANSFER_OVERHEAD_LENGTH; diff --git a/packages/stream-node/reader.js b/packages/stream-node/reader.js index 63c6b429c8..40ca54e3ba 100644 --- a/packages/stream-node/reader.js +++ b/packages/stream-node/reader.js @@ -8,7 +8,7 @@ import { mapReader } from '@endo/stream'; -const { details: X, quote: q } = assert; +const { details: X, Fail, quote: q } = assert; /** * @param {import('stream').Readable} input the source Node.js reader @@ -16,9 +16,7 @@ const { details: X, quote: q } = assert; */ export const makeNodeReader = input => { !input.readableObjectMode || - assert.fail( - X`Cannot convert Node.js object mode Reader to AsyncIterator`, - ); + Fail`Cannot convert Node.js object mode Reader to AsyncIterator`; assert( input.readableEncoding === null, X`Cannot convert Node.js Reader with readableEncoding ${q( diff --git a/packages/stream-node/writer.js b/packages/stream-node/writer.js index 702e9771c7..65c0b8c626 100644 --- a/packages/stream-node/writer.js +++ b/packages/stream-node/writer.js @@ -5,7 +5,7 @@ // @ts-check /// -const { details: X } = assert; +const { Fail } = assert; /** * Adapts a Node.js writable stream to a JavaScript @@ -18,9 +18,7 @@ const { details: X } = assert; */ export const makeNodeWriter = writer => { !writer.writableObjectMode || - assert.fail( - X`Cannot convert Node.js object mode Writer to AsyncIterator`, - ); + Fail`Cannot convert Node.js object mode Writer to AsyncIterator`; const finalIteration = new Promise((resolve, reject) => { const finalize = () => {