From c2b49f6a207dd3a9e062609e827186f8571249ac Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sun, 7 Mar 2021 20:34:57 -0800 Subject: [PATCH 1/2] fix: toStringTag dominates default q stringify --- packages/ses/src/error/stringify-utils.js | 5 +---- packages/ses/test/error/test-assert-log.js | 6 ++---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/ses/src/error/stringify-utils.js b/packages/ses/src/error/stringify-utils.js index 56e769e100..b51aa22e8e 100644 --- a/packages/ses/src/error/stringify-utils.js +++ b/packages/ses/src/error/stringify-utils.js @@ -58,13 +58,10 @@ const bestEffortStringify = (payload, spaces = undefined) => { return '[Seen]'; } seenSet.add(val); - if (Promise.resolve(val) === val) { - return '[Promise]'; - } if (val instanceof Error) { return `[${val.name}: ${val.message}]`; } - if (Object.keys(val).length === 0 && Symbol.toStringTag in val) { + if (Symbol.toStringTag in val) { // Note that this test is `Object.keys` rather than `Refect.ownKeys`. // Like `JSON.stringify`, `Object.ownKeys` will enumerate only // string-named enumerable own properties, which will therefore diff --git a/packages/ses/test/error/test-assert-log.js b/packages/ses/test/error/test-assert-log.js index b66ceb41a8..f433adac1d 100644 --- a/packages/ses/test/error/test-assert-log.js +++ b/packages/ses/test/error/test-assert-log.js @@ -400,7 +400,7 @@ test('q as best efforts stringify', t => { ]; t.is( `${q(challenges)}`, - '["[Promise]","[Function foo]","[[hilbert]]","[undefined]","undefined","[URIError: wut?]",["[33n]","[Symbol(foo)]","[Symbol(bar)]","[Symbol(Symbol.asyncIterator)]"],{"NaN":"[NaN]","Infinity":"[Infinity]","neg":"[-Infinity]"},18014398509481984,{"superTagged":"[Tagged]","subTagged":"[Tagged]","subTaggedNonEmpty":{"foo":"x"}}]', + '["[Promise]","[Function foo]","[[hilbert]]","[undefined]","undefined","[URIError: wut?]",["[33n]","[Symbol(foo)]","[Symbol(bar)]","[Symbol(Symbol.asyncIterator)]"],{"NaN":"[NaN]","Infinity":"[Infinity]","neg":"[-Infinity]"},18014398509481984,{"superTagged":"[Tagged]","subTagged":"[Tagged]","subTaggedNonEmpty":"[Tagged]"}]', ); t.is( `${q(challenges, ' ')}`, @@ -427,9 +427,7 @@ test('q as best efforts stringify', t => { { "superTagged": "[Tagged]", "subTagged": "[Tagged]", - "subTaggedNonEmpty": { - "foo": "x" - } + "subTaggedNonEmpty": "[Tagged]" } ]`, ); From cb54de6c866ab7f377e9f85a48567c7cd4360146 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Mon, 8 Mar 2021 14:28:16 -0800 Subject: [PATCH 2/2] fix: comment review comment --- packages/ses/src/error/stringify-utils.js | 24 +++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/ses/src/error/stringify-utils.js b/packages/ses/src/error/stringify-utils.js index b51aa22e8e..1b9140f318 100644 --- a/packages/ses/src/error/stringify-utils.js +++ b/packages/ses/src/error/stringify-utils.js @@ -62,15 +62,27 @@ const bestEffortStringify = (payload, spaces = undefined) => { return `[${val.name}: ${val.message}]`; } if (Symbol.toStringTag in val) { - // Note that this test is `Object.keys` rather than `Refect.ownKeys`. - // Like `JSON.stringify`, `Object.ownKeys` will enumerate only - // string-named enumerable own properties, which will therefore - // omit Symbol.toStringTag even if it is own and enumerable. - // This case will happen to do a good job with presences without + // For the built-ins that have or inherit a `Symbol.toStringTag`-named + // property, most of them inherit the default `toString` method, + // which will print in a similar manner: `"[object Foo]"` vs + // `"[Foo]"`. The exceptions are + // * `Symbol.prototype`, `BigInt.prototype`, `String.prototype` + // which don't matter to us since we handle primitives + // separately and we don't care about primitive wrapper objects. + // * TODO + // `Date.prototype`, `TypedArray.prototype`. + // Hmmm, we probably should make special cases for these. We're + // not using these yet, so it's not urgent. But others will run + // into these. + // + // Once #2018 is closed, the only objects in our code that have or + // inherit a `Symbol.toStringTag`-named property are remotables + // or their remote presences. + // This printing will do a good job for these without // violating abstraction layering. This behavior makes sense // purely in terms of JavaScript concepts. That's some of the // motivation for choosing that representation of remotables - // in the first place. + // and their remote presences in the first place. return `[${val[Symbol.toStringTag]}]`; } return val;