fix: replace openDetail with quoting q

The new `q` both declassifies, as the old `openDetail` did, as well as
doing a modified `JSON.stringify`, to quote the declassified contents
as placed into the thrown error's message. The unquoted data itself is
still sent to the console for better debugging if the console supports
that.

`q` differs from `JSON.stringify` in being cycle tolerant. It will print
subtrees it has already seen as "<**seen**>", collapsing both dags and
cycles.

packages/ERTP/test/unitTests/test-issuerObj.js

@@ -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) {

packages/assert/src/assert.js

@@ -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
@@ -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)}`,
  * );
  * ```
  *
@@ -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
@@ -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)})`;
         }
@@ -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();
 }
@@ -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);
 }
 
 /**
@@ -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);
 }
 
@@ -226,4 +262,4 @@ assert.fail = fail;
 assert.typeof = assertTypeof;
 harden(assert);
 
-export { assert, details, openDetail, an };
+export { assert, details, q, an };

packages/assert/test/test-assert.js

@@ -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 => {
@@ -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'],
     ]);
@@ -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);

packages/cosmic-swingset/lib/ag-solo/vats/lib-wallet.js

@@ -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';
@@ -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);

packages/same-structure/src/sameStructure.js

@@ -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
@@ -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})`,
     );

packages/store/src/store.js

@@ -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);
 
@@ -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) => {

packages/wallet-frontend/src/utils/fetch-websocket.js

@@ -12,7 +12,7 @@ export async function doFetch(req) {
   }
 
   const socket = websocket;
-  
+
   let resolve;
   const p = new Promise(res => {
     resolve = res;