Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace openDetail with quoting q #1134

Merged
merged 1 commit into from
Jun 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/ERTP/test/unitTests/test-issuerObj.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ test('issuer.combine bad payments', t => {

t.rejects(
() => E(issuer).combine(payments),
/payment not found/,
/"payment" not found/,
'payment from other mint is not found',
);
} catch (e) {
Expand Down
86 changes: 61 additions & 25 deletions packages/assert/src/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,40 @@ function an(str) {
}
harden(an);

/**
* Like `JSON.stringify` but does not blow up if given a cycle. This is not
* intended to be a serialization to support any useful unserialization,
* or any programmatic use of the resulting string. The string is intended
* only for showing a human, in order to be informative enough for some
* logging purposes. As such, this `cycleTolerantStringify` has an
* imprecise specification and may change over time.
*
* The current `cycleTolerantStringify` possibly emits too many "seen"
* markings: Not only for cycles, but also for repeated subtrees by
* object identity.
*/
function cycleTolerantStringify(payload) {
const seenSet = new Set();
const replacer = (_, val) => {
if (typeof val === 'object' && val !== null) {
if (seenSet.has(val)) {
return '<**seen**>';
}
seenSet.add(val);
}
return val;
};
return JSON.stringify(payload, replacer);
}

const declassifiers = new WeakSet();

/**
* To "declassify" a substitution value used in a details`...` template literal,
* enclose that substitution expression in a call to openDetail. This states
* that the argument should appear, stringified, in the error message of the
* thrown error.
* To "declassify" and quote a substitution value used in a
* details`...` template literal, enclose that substitution expression
* in a call to `q`. This states that the argument should appear quoted (with
* `JSON.stringify`), in the error message of the thrown error. The payload
* itself is still passed unquoted to the console as it would be without q.
*
* Starting from the example in the `details` comment, say instead that the
* color the sky is supposed to be is also computed. Say that we still don't
Expand All @@ -41,7 +68,7 @@ const declassifiers = new WeakSet();
* assert.equal(
* sky.color,
* color,
* details`${sky.color} should be ${openDetail(color)}`,
* details`${sky.color} should be ${q(color)}`,
* );
* ```
*
Expand All @@ -52,24 +79,23 @@ const declassifiers = new WeakSet();
* @param {*} payload What to declassify
* @returns {StringablePayload} The declassified payload
*/
function openDetail(payload) {
const result = harden({
function q(payload) {
// Don't harden the payload
const result = Object.freeze({
payload,
toString() {
return payload.toString();
},
toString: Object.freeze(() => cycleTolerantStringify(payload)),
});
declassifiers.add(result);
return result;
}
harden(openDetail);
harden(q);

/**
* Use the `details` function as a template literal tag to create
* informative error messages. The assertion functions take such messages
* as optional arguments:
* ```js
* assert(sky.isBlue(), details`${sky.color} should be blue`);
* assert(sky.isBlue(), details`${sky.color} should be "blue"`);
* ```
* The details template tag returns an object that can print itself with the
* formatted message in two ways. It will report the real details to the
Expand Down Expand Up @@ -112,8 +138,8 @@ function details(template, ...args) {
let arg = args[i];
let argStr;
if (declassifiers.has(arg)) {
arg = arg.payload;
argStr = `${arg}`;
arg = arg.payload;
} else {
argStr = `(${an(typeof arg)})`;
}
Expand Down Expand Up @@ -158,7 +184,9 @@ harden(details);
*/
function fail(optDetails = details`Assert failed`) {
if (typeof optDetails === 'string') {
optDetails = details`${openDetail(optDetails)}`;
// If it is a string, use it as the literal part of the template so
// it doesn't get quoted.
optDetails = details([optDetails]);
}
throw optDetails.complain();
}
Expand All @@ -185,24 +213,24 @@ function fail(optDetails = details`Assert failed`) {
* @param {*} flag The truthy/falsy value
* @param {Details} [optDetails] The details to throw
*/
function assert(flag, optDetails = details`check failed`) {
function assert(flag, optDetails = details`Check failed`) {
if (!flag) {
fail(optDetails);
}
}

/**
* Assert that two values must be `===`.
* Assert that two values must be `Object.is`.
* @param {*} actual The value we received
* @param {*} expected What we wanted
* @param {Details} [optDetails] The details to throw
*/
function equal(
actual,
expected,
optDetails = details`Expected ${actual} === ${expected}`,
optDetails = details`Expected ${actual} is same as ${expected}`,
) {
assert(actual === expected, optDetails);
assert(Object.is(actual, expected), optDetails);
}

/**
Expand All @@ -212,12 +240,20 @@ function equal(
* @param {string} typename The expected name
* @param {Details} [optDetails] The details to throw
*/
function assertTypeof(
specimen,
typename,
optDetails = details`${specimen} must be ${openDetail(an(typename))}`,
) {
assert(typeof typename === 'string', details`${typename} must be a string`);
function assertTypeof(specimen, typename, optDetails) {
assert(
typeof typename === 'string',
details`${q(typename)} must be a string`,
);
if (optDetails === undefined) {
// Like
// ```js
// optDetails = details`${specimen} must be ${q(an(typename))}`;
// ```
// except it puts the typename into the literal part of the template
// so it doesn't get quoted.
optDetails = details(['', ` must be ${an(typename)}`], specimen);
}
equal(typeof specimen, typename, optDetails);
}

Expand All @@ -226,4 +262,4 @@ assert.fail = fail;
assert.typeof = assertTypeof;
harden(assert);

export { assert, details, openDetail, an };
export { assert, details, q, an };
111 changes: 100 additions & 11 deletions packages/assert/test/test-assert.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { test } from 'tape';
import { an, assert, details, openDetail } from '../src/assert';
import { an, assert, details, q } from '../src/assert';
import { throwsAndLogs } from './throwsAndLogs';

test('an', t => {
Expand Down Expand Up @@ -45,16 +45,31 @@ test('throwsAndLogs', t => {
test('assert', t => {
try {
assert(2 + 3 === 5);
assert.equal(2 + 3, 5);
throwsAndLogs(t, () => assert(false), /check failed/, [
['error', 'LOGGED ERROR:', 'check failed'],

throwsAndLogs(t, () => assert(false), /Check failed/, [
['error', 'LOGGED ERROR:', 'Check failed'],
]);
throwsAndLogs(
t,
() => assert.equal(5, 6),
/Expected \(a number\) === \(a number\)/,
[['error', 'LOGGED ERROR:', 'Expected', 5, '===', 6]],
);
throwsAndLogs(t, () => assert(false, 'foo'), /foo/, [
['error', 'LOGGED ERROR:', 'foo'],
]);

throwsAndLogs(t, () => assert.fail(), /Assert failed/, [
['error', 'LOGGED ERROR:', 'Assert failed'],
]);
throwsAndLogs(t, () => assert.fail('foo'), /foo/, [
['error', 'LOGGED ERROR:', 'foo'],
]);
} catch (e) {
console.log('unexpected exception', e);
t.assert(false, e);
} finally {
t.end();
}
});

test('assert equals', t => {
try {
assert.equal(2 + 3, 5);
throwsAndLogs(t, () => assert.equal(5, 6, 'foo'), /foo/, [
['error', 'LOGGED ERROR:', 'foo'],
]);
Expand All @@ -66,10 +81,84 @@ test('assert', t => {
);
throwsAndLogs(
t,
() => assert.equal(5, 6, details`${5} !== ${openDetail(6)}`),
() => assert.equal(5, 6, details`${5} !== ${q(6)}`),
/\(a number\) !== 6/,
[['error', 'LOGGED ERROR:', 5, '!==', 6]],
);

assert.equal(NaN, NaN);
throwsAndLogs(
t,
() => assert.equal(-0, 0),
/Expected \(a number\) is same as \(a number\)/,
[['error', 'LOGGED ERROR:', 'Expected', -0, 'is same as', 0]],
);
} catch (e) {
console.log('unexpected exception', e);
t.assert(false, e);
} finally {
t.end();
}
});

test('assert typeof', t => {
try {
assert.typeof(2, 'number');
throwsAndLogs(
t,
() => assert.typeof(2, 'string'),
/\(a number\) must be a string/,
[['error', 'LOGGED ERROR:', 2, 'must be a string']],
);
throwsAndLogs(t, () => assert.typeof(2, 'string', 'foo'), /foo/, [
['error', 'LOGGED ERROR:', 'foo'],
]);
} catch (e) {
console.log('unexpected exception', e);
t.assert(false, e);
} finally {
t.end();
}
});

test('assert q', t => {
try {
throwsAndLogs(
t,
() => assert.fail(details`<${'bar'},${q('baz')}>`),
/<\(a string\),"baz">/,
[['error', 'LOGGED ERROR:', '<', 'bar', ',', 'baz', '>']],
);

const list = ['a', 'b', 'c'];
throwsAndLogs(
t,
() => assert.fail(details`${q(list)}`),
/\["a","b","c"\]/,
[['error', 'LOGGED ERROR:', ['a', 'b', 'c']]],
);
const repeat = { x: list, y: list };
throwsAndLogs(
t,
() => assert.fail(details`${q(repeat)}`),
/{"x":\["a","b","c"\],"y":"<\*\*seen\*\*>"}/,
[['error', 'LOGGED ERROR:', { x: ['a', 'b', 'c'], y: ['a', 'b', 'c'] }]],
);

// Make it into a cycle
list[1] = list;
throwsAndLogs(
t,
() => assert.fail(details`${q(list)}`),
/\["a","<\*\*seen\*\*>","c"\]/,
[['error', 'LOGGED ERROR:', ['a', list, 'c']]],
);
throwsAndLogs(
t,
() => assert.fail(details`${q(repeat)}`),
/{"x":\["a","<\*\*seen\*\*>","c"\],"y":"<\*\*seen\*\*>"}/,
[['error', 'LOGGED ERROR:', { x: list, y: ['a', list, 'c'] }]],
);
} catch (e) {
console.log('unexpected exception', e);
t.assert(false, e);
Expand Down
6 changes: 2 additions & 4 deletions packages/cosmic-swingset/lib/ag-solo/vats/lib-wallet.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import harden from '@agoric/harden';
import { assert, details, openDetail } from '@agoric/assert';
import { assert, details, q } from '@agoric/assert';
import makeStore from '@agoric/store';
import makeWeakStore from '@agoric/weak-store';
import makeAmountMath from '@agoric/ertp/src/amountMath';
Expand Down Expand Up @@ -291,9 +291,7 @@ export async function makeWallet({
const makeEmptyPurse = async (brandPetname, petnameForPurse) => {
assert(
!purseMapping.petnameToVal.has(petnameForPurse),
details`Purse petname ${openDetail(
petnameForPurse,
)} already used in wallet.`,
details`Purse petname ${q(petnameForPurse)} already used in wallet.`,
);
const brand = brandMapping.petnameToVal.get(brandPetname);
const { issuer } = brandTable.get(brand);
Expand Down
4 changes: 2 additions & 2 deletions packages/same-structure/src/sameStructure.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import harden from '@agoric/harden';
import { sameValueZero, passStyleOf } from '@agoric/marshal';
import { assert, details, openDetail } from '@agoric/assert';
import { assert, details, q } from '@agoric/assert';

// Shim of Object.fromEntries from
// https://github.com/tc39/proposal-object-from-entries/blob/master/polyfill.js
Expand Down Expand Up @@ -166,7 +166,7 @@ function pathStr(path) {
function mustBeSameStructureInternal(left, right, message, path) {
function complain(problem) {
assert.fail(
details`${openDetail(message)}: ${openDetail(problem)} at ${openDetail(
details`${q(message)}: ${q(problem)} at ${q(
pathStr(path),
)}: (${left}) vs (${right})`,
);
Expand Down
9 changes: 3 additions & 6 deletions packages/store/src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// @ts-check

import rawHarden from '@agoric/harden';
import { assert, details, openDetail } from '@agoric/assert';
import { assert, details, q } from '@agoric/assert';

const harden = /** @type {<T>(x: T) => T} */ (rawHarden);

Expand Down Expand Up @@ -32,12 +32,9 @@ const harden = /** @type {<T>(x: T) => T} */ (rawHarden);
function makeStore(keyName = 'key') {
const store = new Map();
const assertKeyDoesNotExist = key =>
assert(
!store.has(key),
details`${openDetail(keyName)} already registered: ${key}`,
);
assert(!store.has(key), details`${q(keyName)} already registered: ${key}`);
const assertKeyExists = key =>
assert(store.has(key), details`${openDetail(keyName)} not found: ${key}`);
assert(store.has(key), details`${q(keyName)} not found: ${key}`);
return harden({
has: key => store.has(key),
init: (key, value) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/wallet-frontend/src/utils/fetch-websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export async function doFetch(req) {
}

const socket = websocket;

let resolve;
const p = new Promise(res => {
resolve = res;
Expand Down
Loading