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

Best effort assert printing #772

Merged
merged 8 commits into from
Jun 9, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 48 additions & 27 deletions packages/ses/src/error/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,50 @@ freeze(quote);
*/
const hiddenDetailsMap = new WeakMap();

/**
* @param {HiddenDetails} hiddenDetails
* @returns {string}
*/
const getMessageString = ({ template, args }) => {
erights marked this conversation as resolved.
Show resolved Hide resolved
const parts = [template[0]];
for (let i = 0; i < args.length; i += 1) {
const arg = args[i];
let argStr;
if (declassifiers.has(arg)) {
argStr = `${arg}`;
} else if (arg instanceof Error) {
argStr = `(${an(arg.name)})`;
} else {
argStr = `(${an(typeof arg)})`;
}
parts.push(argStr, template[i + 1]);
}
return parts.join('');
};

/**
* Give detailsTokens a toString behavior. To minimize the overhead of
* creating new detailsTokens, we do this with an
* inherited `this` sensitive `toString` method, even though we normally
* avoid `this` sensitivity. To protect the method from inappropriate
* `this` application, it does something interesting only for objects
* registered in `redactedDetails`, which should be exactly the detailsTokens.
*
* The printing behavior must not reveal anything redacted, so we just use
* the same `getMessageString` we use to construct the redacted message
* string for a thrown assertion error.
*/
const DetailsTokenProto = freeze({
toString() {
const hiddenDetails = hiddenDetailsMap.get(this);
if (hiddenDetails === undefined) {
return '[Not a DetailsToken]';
}
return getMessageString(hiddenDetails);
},
});
freeze(DetailsTokenProto.toString);

/**
* Normally this is the function exported as `assert.details` and often
* spelled `d`. However, if the `{errorTaming: 'unsafe'}` option is given to
Expand All @@ -70,7 +114,7 @@ const redactedDetails = (template, ...args) => {
// a details token that is never used, so this path must remain as fast as
// possible. Hence we store what we've got with little processing, postponing
// all the work to happen only if needed, for example, if an assertion fails.
const detailsToken = freeze({ __proto__: null });
const detailsToken = freeze({ __proto__: DetailsTokenProto });
hiddenDetailsMap.set(detailsToken, { template, args });
return detailsToken;
};
Expand All @@ -90,35 +134,12 @@ freeze(redactedDetails);
* @type {DetailsTag}
*/
const unredactedDetails = (template, ...args) => {
const detailsToken = freeze({ __proto__: null });
args = args.map(arg => (declassifiers.has(arg) ? arg : quote(arg)));
hiddenDetailsMap.set(detailsToken, { template, args });
return detailsToken;
return redactedDetails(template, ...args);
};
freeze(unredactedDetails);
export { unredactedDetails };

/**
* @param {HiddenDetails} hiddenDetails
* @returns {string}
*/
const getMessageString = ({ template, args }) => {
const parts = [template[0]];
for (let i = 0; i < args.length; i += 1) {
const arg = args[i];
let argStr;
if (declassifiers.has(arg)) {
argStr = `${arg}`;
} else if (arg instanceof Error) {
argStr = `(${an(arg.name)})`;
} else {
argStr = `(${an(typeof arg)})`;
}
parts.push(argStr, template[i + 1]);
}
return parts.join('');
};

/**
* @param {HiddenDetails} hiddenDetails
* @returns {LogArgs}
Expand Down Expand Up @@ -193,7 +214,7 @@ const makeError = (
}
const hiddenDetails = hiddenDetailsMap.get(optDetails);
if (hiddenDetails === undefined) {
throw new Error(`unrecognized details ${optDetails}`);
throw new Error(`unrecognized details ${quote(optDetails)}`);
}
const messageString = getMessageString(hiddenDetails);
const error = new ErrorConstructor(messageString);
Expand Down Expand Up @@ -243,7 +264,7 @@ const note = (error, detailsNote) => {
}
const hiddenDetails = hiddenDetailsMap.get(detailsNote);
if (hiddenDetails === undefined) {
throw new Error(`unrecognized details ${detailsNote}`);
throw new Error(`unrecognized details ${quote(detailsNote)}`);
}
const logArgs = getLogArgs(hiddenDetails);
const callbacks = hiddenNoteCallbackArrays.get(error);
Expand Down
12 changes: 11 additions & 1 deletion packages/ses/src/error/stringify-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,17 @@ const bestEffortStringify = (payload, spaces = undefined) => {
}
}
};
return JSON.stringify(payload, replacer, spaces);
try {
return JSON.stringify(payload, replacer, spaces);
} catch (_err) {
// Don't do anything more fancy here if there is any
// chance that might throw, unless you surround that
// with another try-catch-recovery. For example,
// the caught thing might be a proxy or other exotic
// object rather than an error. The proxy might throw
// whenever it is possible for it to.
return '[Something that failed to stringify]';
}
};
freeze(bestEffortStringify);
export { bestEffortStringify };
39 changes: 37 additions & 2 deletions packages/ses/test/error/test-assert-log.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,10 +426,11 @@ test('q as best efforts stringify', t => {
},
2 ** 54,
{ superTagged, subTagged, subTaggedNonEmpty },
{ __proto__: null },
];
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":"[Tagged]"}]',
'["[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, ' ')}`,
Expand Down Expand Up @@ -457,7 +458,41 @@ test('q as best efforts stringify', t => {
"superTagged": "[Tagged]",
"subTagged": "[Tagged]",
"subTaggedNonEmpty": "[Tagged]"
}
},
{}
]`,
);
});

// See https://github.com/endojs/endo/issues/729
test('printing detailsToken', t => {
t.throws(() => assert.error({ __proto__: null }), {
message: 'unrecognized details {}',
});
});

test('q tolerates always throwing exotic', t => {
/**
* alwaysThrowHandler
* This is an object that throws if any propery is read. It's used as
* a proxy handler which throws on any trap called.
* It's made from a proxy with a get trap that throws.
*/
const alwaysThrowHandler = new Proxy(
{ __proto__: null },
{
get(_shadow, _prop) {
throw Error('Always throw');
},
},
);

/**
* A proxy that throws on any trap, i.e., the proxy throws whenever in can
* throw. Potentially useful in many other tests. TODO put somewhere reusable
* by other tests.
*/
const alwaysThrowProxy = new Proxy({ __proto__: null }, alwaysThrowHandler);

t.is(`${q(alwaysThrowProxy)}`, '[Something that failed to stringify]');
});