Skip to content

Commit

Permalink
refactor: use @endo/errors when acyclic (#2324)
Browse files Browse the repository at this point in the history
Staged on #2323 

Closes: #XXXX
Refs: Agoric/agoric-sdk#5672
Agoric/agoric-sdk#9513

## Description

Does for endo what Agoric/agoric-sdk#9513 does
for agoric-sdk --- importing `assert` from @endo/errors --- when it can
do so without causing dependency cycles. For the remaining packages that
would cause dependency cycles, just move them towards more modern assert
conventions while still using the unstructured global `assert`

### Security Considerations

none
### Scaling Considerations

none
### Documentation Considerations

We should document `assert` only as imported from @endo/errors, since
our users will not generally contribute code within endo's internal
dependency cycles.

### Testing Considerations

The CI run on this PR also serves to test #2323, since that one, by
itself, does not cause a behavioral change. It's purpose is to enable
this PR not to hit the problem described at
Agoric/agoric-sdk#9515

### Compatibility Considerations

none
### Upgrade Considerations

none
  • Loading branch information
erights authored Jun 17, 2024
1 parent 8b2bedb commit 74db1e8
Show file tree
Hide file tree
Showing 36 changed files with 138 additions and 146 deletions.
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@endo/bundle-source": "^3.2.3",
"@endo/compartment-mapper": "^1.1.5",
"@endo/daemon": "^2.3.0",
"@endo/errors": "^1.2.2",
"@endo/eventual-send": "^1.2.2",
"@endo/exo": "^1.5.0",
"@endo/far": "^1.1.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/pet-name.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { quote: q } = assert;
import { q } from '@endo/errors';

/**
* Splits a dot-delimited pet name path into an array of pet names.
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/daemon-node-powers.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { makePromiseKit } from '@endo/promise-kit';
import { makePipe } from '@endo/stream';
import { makeNodeReader, makeNodeWriter } from '@endo/stream-node';
import { q } from '@endo/errors';
import { makeNetstringCapTP } from './connection.js';
import { makeReaderRef } from './reader-ref.js';
import { makePetStoreMaker } from './pet-store.js';
Expand All @@ -14,8 +15,6 @@ import { makeSerialJobs } from './serial-jobs.js';
/** @import { ERef, FarRef } from '@endo/eventual-send' */
/** @import { Config, CryptoPowers, DaemonWorkerFacet, DaemonicPersistencePowers, DaemonicPowers, EndoReadable, FilePowers, Formula, NetworkPowers, SocketPowers, WorkerDaemonFacet } from './types.js' */

const { quote: q } = assert;

const textEncoder = new TextEncoder();

/**
Expand Down
12 changes: 5 additions & 7 deletions packages/daemon/src/daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { makeExo } from '@endo/exo';
import { E, Far } from '@endo/far';
import { makeMarshal } from '@endo/marshal';
import { makePromiseKit } from '@endo/promise-kit';
import { q } from '@endo/errors';
import { makeError, q, X } from '@endo/errors';
import { makeRefReader } from './ref-reader.js';
import { makeDirectoryMaker } from './directory.js';
import { makeMailboxMaker } from './mail.js';
Expand Down Expand Up @@ -491,7 +491,7 @@ const makeDaemonCore = async (
const mustGetIdForRef = ref => {
const id = idForRef.get(ref);
if (id === undefined) {
throw assert.error(assert.details`No corresponding formula for ${ref}`);
throw makeError(X`No corresponding formula for ${ref}`);
}
return id;
};
Expand All @@ -501,11 +501,9 @@ const makeDaemonCore = async (
const ref = refForId.get(id);
if (ref === undefined) {
if (formulaForId.get(id) !== undefined) {
throw assert.error(
assert.details`Formula has not produced a ref ${id}`,
);
throw makeError(X`Formula has not produced a ref ${id}`);
}
throw assert.error(assert.details`Unknown identifier ${id}`);
throw makeError(X`Unknown identifier ${id}`);
}
return ref;
};
Expand Down Expand Up @@ -1576,7 +1574,7 @@ const makeDaemonCore = async (
const guestNodeNumber = url.hostname;

if (!guestHandleNumber) {
throw assert.error('Handle locator must have an "id" parameter');
throw makeError('Handle locator must have an "id" parameter');
}

const guestHandleId = formatId({
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/directory.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@

import { E } from '@endo/far';
import { makeExo } from '@endo/exo';
import { q } from '@endo/errors';
import { makeIteratorRef } from './reader-ref.js';
import { formatLocator, idFromLocator } from './locator.js';

import { DirectoryInterface } from './interfaces.js';

const { quote: q } = assert;

/** @import { DaemonCore, MakeDirectoryNode, EndoDirectory, NameHub, LocatorNameChange, Context } from './types.js' */

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/daemon/src/formula-identifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

/** @import { IdRecord } from './types.js' */

const { quote: q } = assert;
import { makeError, q } from '@endo/errors';

const numberPattern = /^[0-9a-f]{128}$/;
const idPattern = /^(?<number>[0-9a-f]{128}):(?<node>[0-9a-f]{128})$/;
Expand All @@ -19,7 +19,7 @@ export const isValidNumber = allegedNumber =>
*/
export const assertValidNumber = allegedNumber => {
if (!isValidNumber(allegedNumber)) {
throw assert.error(`Invalid number ${q(allegedNumber)}`);
throw makeError(`Invalid number ${q(allegedNumber)}`);
}
};

Expand All @@ -45,11 +45,11 @@ export const assertValidId = (id, petName) => {
export const parseId = id => {
const match = idPattern.exec(id);
if (match === null) {
throw assert.error(`Invalid formula identifier ${q(id)}`);
throw makeError(`Invalid formula identifier ${q(id)}`);
}
const { groups } = match;
if (groups === undefined) {
throw assert.error(
throw makeError(
`Programmer invariant failure: expected match groups, formula identifier was ${q(
id,
)}`,
Expand Down
2 changes: 1 addition & 1 deletion packages/daemon/src/formula-type.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @ts-check

const { quote: q } = assert;
import { q } from '@endo/errors';

// Note: Alphabetically sorted
const formulaTypes = new Set([
Expand Down
7 changes: 3 additions & 4 deletions packages/daemon/src/host.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { E } from '@endo/far';
import { makeExo } from '@endo/exo';
import { makeError, q } from '@endo/errors';
import { makeIteratorRef } from './reader-ref.js';
import { assertPetName, petNamePathFrom } from './pet-name.js';
import { parseId, formatId } from './formula-identifier.js';
Expand All @@ -14,8 +15,6 @@ import { makeDeferredTasks } from './deferred-tasks.js';

import { HostInterface } from './interfaces.js';

const { quote: q } = assert;

/** @param {string} name */
const assertPowersName = name => {
['NONE', 'AGENT', 'ENDO'].includes(name) || assertPetName(name);
Expand Down Expand Up @@ -494,10 +493,10 @@ export const makeHostMaker = ({

nodeNumber || assert.Fail`Invitation must have a hostname`;
if (!remoteHandleNumber) {
throw assert.error(`Invitation must have a "from" parameter`);
throw makeError(`Invitation must have a "from" parameter`);
}
if (invitationNumber === null) {
throw assert.error(`Invitation must have an "id" parameter`);
throw makeError(`Invitation must have an "id" parameter`);
}

/** @type {PeerInfo} */
Expand Down
17 changes: 8 additions & 9 deletions packages/daemon/src/locator.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// @ts-check

import { makeError, q } from '@endo/errors';
import { formatId, isValidNumber, parseId } from './formula-identifier.js';
import { isValidFormulaType } from './formula-type.js';

const { quote: q } = assert;

/**
* The endo locator format:
* ```
Expand All @@ -28,7 +27,7 @@ const isValidLocatorType = allegedType =>
*/
const assertValidLocatorType = allegedType => {
if (!isValidLocatorType(allegedType)) {
throw assert.error(`Unrecognized locator type ${q(allegedType)}`);
throw makeError(`Unrecognized locator type ${q(allegedType)}`);
}
};

Expand All @@ -40,35 +39,35 @@ export const parseLocator = allegedLocator => {
const errorPrefix = `Invalid locator ${q(allegedLocator)}:`;

if (!URL.canParse(allegedLocator)) {
throw assert.error(`${errorPrefix} Invalid URL.`);
throw makeError(`${errorPrefix} Invalid URL.`);
}
const url = new URL(allegedLocator);

if (!allegedLocator.startsWith('endo://')) {
throw assert.error(`${errorPrefix} Invalid protocol.`);
throw makeError(`${errorPrefix} Invalid protocol.`);
}

const node = url.host;
if (!isValidNumber(node)) {
throw assert.error(`${errorPrefix} Invalid node identifier.`);
throw makeError(`${errorPrefix} Invalid node identifier.`);
}

if (
url.searchParams.size !== 2 ||
!url.searchParams.has('id') ||
!url.searchParams.has('type')
) {
throw assert.error(`${errorPrefix} Invalid search params.`);
throw makeError(`${errorPrefix} Invalid search params.`);
}

const number = url.searchParams.get('id');
if (number === null || !isValidNumber(number)) {
throw assert.error(`${errorPrefix} Invalid id.`);
throw makeError(`${errorPrefix} Invalid id.`);
}

const formulaType = url.searchParams.get('type');
if (formulaType === null || !isValidLocatorType(formulaType)) {
throw assert.error(`${errorPrefix} Invalid type.`);
throw makeError(`${errorPrefix} Invalid type.`);
}

return { formulaType, node, number };
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/mail.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { E } from '@endo/eventual-send';
import { makeExo } from '@endo/exo';
import { makePromiseKit } from '@endo/promise-kit';
import { q } from '@endo/errors';
import { makeChangeTopic } from './pubsub.js';
import { assertPetName } from './pet-name.js';

Expand All @@ -17,8 +18,6 @@ import {
/** @import { PromiseKit } from '@endo/promise-kit' */
/** @import { Envelope, EnvelopedMessage, Handle, Mail, MakeMailbox, Provide, Request, StampedMessage, Topic } from './types.js' */

const { quote: q } = assert;

/**
* @param {string} description
* @param {string} fromId
Expand Down
2 changes: 1 addition & 1 deletion packages/daemon/src/multimap.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/** @import { Multimap, WeakMultimap, BidirectionalMultimap } from './types.js' */

const { quote: q } = assert;
import { q } from '@endo/errors';

/**
* @param {new () => (Map | WeakMap)} mapConstructor
Expand Down
2 changes: 1 addition & 1 deletion packages/daemon/src/pet-name.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @ts-check

const { quote: q } = assert;
import { q } from '@endo/errors';

const validNamePattern = /^[a-z][a-z0-9-]{0,127}$/;

Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/pet-sitter.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// @ts-check

import { q } from '@endo/errors';
import { isPetName } from './pet-name.js';
import { parseId } from './formula-identifier.js';

/** @import { PetStore, IdRecord, PetStoreIdNameChange } from './types.js' */

const { quote: q } = assert;

/**
* @param {PetStore} petStore
* @param {Record<string,string>} specialNames
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/pet-store.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
// @ts-check

import { q } from '@endo/errors';
import { makeChangeTopic } from './pubsub.js';
import { parseId, assertValidId, isValidNumber } from './formula-identifier.js';
import { makeBidirectionalMultimap } from './multimap.js';

/** @import { BidirectionalMultimap, Config, FilePowers, IdChangesTopic, NameChangesTopic, PetStore, PetStoreIdNameChange, PetStoreNameChange, PetStorePowers } from './types.js' */

const { quote: q } = assert;

/**
* @param {FilePowers} filePowers
* @param {Config} config
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/serve-private-path.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// @ts-check

import { q } from '@endo/errors';
import { makeNetstringCapTP } from './connection.js';

const { quote: q } = assert;

export const servePrivatePath = (
sockPath,
endoBootstrap,
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/serve-private-port-http.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

import { E } from '@endo/far';
import { mapReader, mapWriter } from '@endo/stream';
import { q } from '@endo/errors';
import {
makeMessageCapTP,
messageToBytes,
bytesToMessage,
} from './connection.js';

const { quote: q } = assert;

export const servePrivatePortHttp = (
requestedWebletPort,
endoBootstrap,
Expand Down
3 changes: 1 addition & 2 deletions packages/daemon/src/web-server-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { mapWriter, mapReader } from '@endo/stream';
import { E, Far } from '@endo/far';
import { makeBundle } from '@endo/compartment-mapper/bundle.js';

import { q } from '@endo/errors';
import { makeHttpPowers } from './web-server-node-powers.js';

import {
Expand All @@ -20,8 +21,6 @@ import {
bytesToMessage,
} from './connection.js';

const { quote: q } = assert;

const { servePortHttp } = makeHttpPowers({ ws, http });

const read = async location => fs.promises.readFile(fileURLToPath(location));
Expand Down
2 changes: 1 addition & 1 deletion packages/daemon/test/move-hub.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { makeExo } from '@endo/exo';
import { M } from '@endo/patterns';

const { quote: q } = assert;
import { q } from '@endo/errors';

/** @import {NameHub} from '../src/types.js' */

Expand Down
4 changes: 2 additions & 2 deletions packages/eventual-send/src/E.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { trackTurns } from './track-turns.js';
import { makeMessageBreakpointTester } from './message-breakpoints.js';

const { details: X, quote: q, Fail } = assert;
const { details: X, quote: q, Fail, error: makeError } = assert;
const { assign, create } = Object;

const onSend = makeMessageBreakpointTester('ENDO_SEND_BREAKPOINTS');
Expand Down Expand Up @@ -52,7 +52,7 @@ const makeEProxyHandler = (recipient, HandledPromise) =>
if (this !== receiver) {
// Reject the async function call
return HandledPromise.reject(
assert.error(
makeError(
X`Unexpected receiver for "${q(propertyKey)}" method of E(${q(
recipient,
)})`,
Expand Down
4 changes: 2 additions & 2 deletions packages/eventual-send/src/handled-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from './local.js';
import { makePostponedHandler } from './postponed.js';

const { Fail, details: X, quote: q } = assert;
const { Fail, details: X, quote: q, note: annotateError } = assert;

const {
create,
Expand Down Expand Up @@ -337,7 +337,7 @@ export const makeHandledPromise = () => {
handledResolve(resolvedTarget);
return resolvedTarget;
} catch (e) {
assert.note(e, X`during resolveWithPresence`);
annotateError(e, X`during resolveWithPresence`);
handledReject(e);
throw e;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/ses/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ User-visible changes in SES:
for an example of performing a scopeProxy leak workaround.
- Under the default `{errorTaming: 'safe'}` setting, the SES shim already redacts stack traces from error instances when it can (currently: v8, spiderMonkey, XS). The setting `{errorTaming: 'unsafe'}` suppresses that redaction, instead blabbing these stack traces on error instances via the expected `errorInstance.stack`.

The purpose of the `details` template literal tag (often spelled `X` or `d`) together with the `quote` function (often spelled `q`) is to redact data from the error messages carried by error instances. With this release, the same `{errorTaming: 'unsafe'}` would suppress that redaction as well, so that all substitution values would act like they've been quoted. IOW, with this setting
The purpose of the `details` template literal tag (often spelled `X`) together with the `quote` function (often spelled `q`) is to redact data from the error messages carried by error instances. With this release, the same `{errorTaming: 'unsafe'}` would suppress that redaction as well, so that all substitution values would act like they've been quoted. IOW, with this setting

```js
assert(false, X`literal part ${secretData} with ${q(publicData)}.`);
Expand Down
2 changes: 1 addition & 1 deletion packages/ses/docs/lockdown.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ can find it there. When the information leak is tolerable, the `'unsafe'`
setting will preserve the filtered stack information on the `err.stack`.

Like hiding the stack, the purpose of the `details` template literal tag (often
spelled `X` or `d`) together with the `quote` function (often spelled `q`) is
spelled `X`) together with the `quote` function (often spelled `q`) is
to redact data from the error messages carried by error instances. The same
`{errorTaming: 'unsafe'}` suppresses that redaction as well, so that all
substitution values would act like they've been quoted. With this setting
Expand Down
Loading

0 comments on commit 74db1e8

Please sign in to comment.