Bug 1769849 - Improve ShadowRealm.evaluate errors r=evilpie

Conditionally attempt to extract .name and .message and use for issuing error.

Differential Revision: https://phabricator.services.mozilla.com/D151849
This commit is contained in:
Matthew Gaudet 2022-07-20 20:12:00 +00:00
Родитель 67d5545eac
Коммит 43f9e6d9e3
3 изменённых файлов: 76 добавлений и 15 удалений

Просмотреть файл

@ -789,7 +789,8 @@ MSG_DEF(JSMSG_NOT_SHADOW_REALM, 0, JSEXN_TYPEERR, "Object is
MSG_DEF(JSMSG_SHADOW_REALM_EVALUATE_NOT_STRING, 0, JSEXN_TYPEERR, "a ShadowRealm can only evaluate a string")
MSG_DEF(JSMSG_SHADOW_REALM_INVALID_RETURN, 0, JSEXN_TYPEERR, "return value not primitive or callable")
MSG_DEF(JSMSG_SHADOW_REALM_WRAP_FAILURE, 0, JSEXN_TYPEERR, "unable to wrap callable return object")
MSG_DEF(JSMSG_SHADOW_REALM_EVALUATE_FAILURE, 0, JSEXN_TYPEERR, "evaluate failed")
MSG_DEF(JSMSG_SHADOW_REALM_EVALUATE_FAILURE, 0, JSEXN_TYPEERR, "evaluate failed.")
MSG_DEF(JSMSG_SHADOW_REALM_EVALUATE_FAILURE_DETAIL, 1, JSEXN_TYPEERR, "evaluate failed, error was {0}")
MSG_DEF(JSMSG_SHADOW_REALM_WRAPPED_EXECUTION_FAILURE, 0, JSEXN_TYPEERR, "wrapped function threw")
MSG_DEF(JSMSG_SHADOW_REALM_EXPORT_NOT_STRING, 0, JSEXN_TYPEERR, "exportName must be a string")

Просмотреть файл

@ -27,8 +27,10 @@
#include "js/StableStringChars.h"
#include "js/StructuredClone.h"
#include "js/TypeDecls.h"
#include "js/Utility.h"
#include "js/Wrapper.h"
#include "vm/GlobalObject.h"
#include "vm/JSObject.h"
#include "vm/ObjectOperations.h"
#include "builtin/HandlerFunction-inl.h"
@ -166,6 +168,27 @@ static ShadowRealmObject* ValidateShadowRealmObject(JSContext* cx,
return &maybeUnwrappedO->as<ShadowRealmObject>();
}
static void ReportPotentiallyDetailedMessage(JSContext* cx,
const unsigned detailedError,
const unsigned genericError) {
Rooted<Value> exception(cx);
if (!JS_GetPendingException(cx, &exception)) {
return;
}
JS_ClearPendingException(cx);
JS::ErrorReportBuilder jsReport(cx);
JS::ExceptionStack exnStack(cx, exception, nullptr);
if (!jsReport.init(cx, exnStack, JS::ErrorReportBuilder::NoSideEffects)) {
JS_ClearPendingException(cx);
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, genericError);
return;
}
JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, detailedError,
jsReport.toStringResult().c_str());
}
// PerformShadowRealmEval ( sourceText: a String, callerRealm: a Realm Record,
// evalRealm: a Realm Record, )
//
@ -303,21 +326,10 @@ static bool PerformShadowRealmEval(JSContext* cx, HandleString sourceText,
//
// The type error here needs to come from the calling global, so has to
// happen outside the AutoRealm above.
ReportPotentiallyDetailedMessage(cx,
JSMSG_SHADOW_REALM_EVALUATE_FAILURE_DETAIL,
JSMSG_SHADOW_REALM_EVALUATE_FAILURE);
// MG:XXX: Figure out how to extract the error message and include in
// message of TypeError (if possible): See discussion in
// https://github.com/tc39/proposal-shadowrealm/issues/353 for some
// potential pitfalls (i.e. what if the error has a getter on the message
// property?)
//
// I imagine we could do something like GetPropertyPure, and have a nice
// error message if we *don't* have anything to worry about.
//
// https://bugzilla.mozilla.org/show_bug.cgi?id=1769849
JS_ClearPendingException(cx);
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
JSMSG_SHADOW_REALM_EVALUATE_FAILURE);
return false;
}

Просмотреть файл

@ -0,0 +1,48 @@
// |reftest| shell-option(--enable-shadow-realms) skip-if(!xulRuntime.shell)
let sr = new ShadowRealm();
try {
sr.evaluate("throw new Error('hi')");
assertEq(true, false, "Should have thrown");
} catch (e) {
assertEq(e instanceof TypeError, true, "Correct type of error")
assertEq(/Error: hi/.test(e.message), true, "Should have included information from thrown error");
}
try {
sr.evaluate("throw new Error('∂å∂')");
assertEq(true, false, "Should have thrown");
} catch (e) {
assertEq(e instanceof TypeError, true, "Correct type of error")
assertEq(/Error: ∂å∂/.test(e.message), true, "Should have included information from thrown error, UTF-8 Pass through.");
}
try {
sr.evaluate("throw {name: 'Hello', message: 'goodbye'}");
assertEq(true, false, "Should have thrown");
} catch (e) {
assertEq(e instanceof TypeError, true, "Correct type of error")
assertEq(/uncaught exception: Object/.test(e.message), true, "Should get generic fillin message, non-string");
}
try {
sr.evaluate("throw {name: 10, message: 11}");
assertEq(true, false, "Should have thrown");
} catch (e) {
assertEq(e instanceof TypeError, true, "Correct type of error")
assertEq(/uncaught exception: Object/.test(e.message), true, "Should get generic fillin message, non-string");
}
try {
sr.evaluate("throw { get name() { return 'holy'; }, get message() { return 'smokes' } }");
assertEq(true, false, "Should have thrown");
} catch (e) {
assertEq(e instanceof TypeError, true, "Correct type of error")
assertEq(/uncaught exception: Object/.test(e.message), true, "Should get generic error message, getters");
}
if (typeof reportCompare === 'function')
reportCompare(true, true);