Bug 1278562 - DebuggerObject.promiseValue/Reason should throw if promise is not fulfilled/rejected. r=jimb
authorEddy Bruel <ejpbruel@mozilla.com
Fri, 02 Sep 2016 13:24:29 +0200
changeset 312407 fc4928fc74e5add861fb00c40ee40ba5386c82d3
parent 312406 de45e910de91b18ed189a0bdbcb7c9b68aeb39b3
child 312408 ee21ab5cb4796059a8136f70b8dab2371e4d6213
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1278562
milestone51.0a1
Bug 1278562 - DebuggerObject.promiseValue/Reason should throw if promise is not fulfilled/rejected. r=jimb
js/src/doc/Debugger/Debugger.Object.md
js/src/jit-test/tests/debug/inspect-wrapped-promise.js
js/src/js.msg
js/src/vm/Debugger.cpp
--- a/js/src/doc/Debugger/Debugger.Object.md
+++ b/js/src/doc/Debugger/Debugger.Object.md
@@ -213,27 +213,28 @@ from its prototype:
 
     * `"fulfilled"`, if the [`Promise`][promise] has been fulfilled.
 
     * `"rejected"`, if the [`Promise`][promise] has been rejected.
 
     If the referent is not a [`Promise`][promise], throw a `TypeError`.
 
 `promiseValue`
-:   If the referent is a [`Promise`][promise] that has been fulfilled, return
-    a debuggee value representing the value the [`Promise`][promise] has been
-    fulfilled with.
+:   Return a debuggee value representing the value the [`Promise`][promise] has
+    been fulfilled with.
 
-    If the [`Promise`][promise] has not been fulfilled, return `undefined`. If
-    the referent is not a [`Promise`][promise], throw a `TypeError`.
+    If the referent is not a [`Promise`][promise], or the [`Promise`][promise]
+    has not been fulfilled, throw a `TypeError`.
 
 `promiseReason`
-:   If the referent is a [`Promise`][promise] that has been rejected, return a
-    debuggee value representing the value the [`Promise`][promise] has been
-    rejected with.
+:   Return a debuggee value representing the value the [`Promise`][promise] has
+    been rejected with.
+
+    If the referent is not a [`Promise`][promise], or the [`Promise`][promise]
+    has not been rejected, throw a `TypeError`.
 
 `promiseAllocationSite`
 :   If the referent is a [`Promise`][promise], this is the
     [JavaScript execution stack][saved-frame] captured at the time of the
     promise's allocation. This can return null if the promise was not
     created from script. If the referent is not a [`Promise`][promise], throw
     a `TypeError` exception.
 
--- a/js/src/jit-test/tests/debug/inspect-wrapped-promise.js
+++ b/js/src/jit-test/tests/debug/inspect-wrapped-promise.js
@@ -26,24 +26,24 @@ assertEq(promiseDO4.isPromise, false);
 assertEq(promiseDO5.isPromise, false);
 
 assertEq(promiseDO1.promiseState, "pending");
 assertEq(promiseDO2.promiseState, "fulfilled");
 assertEq(promiseDO3.promiseState, "rejected");
 assertThrowsInstanceOf(function () { promiseDO4.promiseState }, TypeError);
 assertThrowsInstanceOf(function () { promiseDO5.promiseState }, TypeError);
 
-assertEq(promiseDO1.promiseValue, undefined);
+assertThrowsInstanceOf(function () { promiseDO1.promiseValue }, TypeError);
 assertEq(promiseDO2.promiseValue, 42);
-assertEq(promiseDO3.promiseValue, undefined);
+assertThrowsInstanceOf(function () { promiseDO3.promiseValue }, TypeError);
 assertThrowsInstanceOf(function () { promiseDO4.promiseValue }, TypeError);
 assertThrowsInstanceOf(function () { promiseDO5.promiseValue }, TypeError);
 
-assertEq(promiseDO1.promiseReason, undefined);
-assertEq(promiseDO2.promiseReason, undefined);
+assertThrowsInstanceOf(function () { promiseDO1.promiseReason }, TypeError);
+assertThrowsInstanceOf(function () { promiseDO2.promiseReason }, TypeError);
 assertEq(promiseDO3.promiseReason, 42);
 assertThrowsInstanceOf(function () { promiseDO4.promiseReason }, TypeError);
 assertThrowsInstanceOf(function () { promiseDO5.promiseReason }, TypeError);
 
 // Depending on whether async stacks are activated, this can be null, which
 // has typeof null.
 assertEq(typeof promiseDO1.promiseAllocationSite === "object", true);
 assertEq(typeof promiseDO2.promiseAllocationSite === "object", true);
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -437,16 +437,18 @@ MSG_DEF(JSMSG_NOT_CALLABLE_OR_UNDEFINED,
 MSG_DEF(JSMSG_NOT_TRACKING_ALLOCATIONS, 1, JSEXN_ERR, "Cannot call {0} without setting trackingAllocationSites to true")
 MSG_DEF(JSMSG_OBJECT_METADATA_CALLBACK_ALREADY_SET, 0, JSEXN_ERR, "Cannot track object allocation, because other tools are already doing so")
 MSG_DEF(JSMSG_QUERY_INNERMOST_WITHOUT_LINE_URL, 0, JSEXN_TYPEERR, "findScripts query object with 'innermost' property must have 'line' and either 'displayURL', 'url', or 'source'")
 MSG_DEF(JSMSG_QUERY_LINE_WITHOUT_URL, 0, JSEXN_TYPEERR, "findScripts query object has 'line' property, but no 'displayURL', 'url', or 'source' property")
 MSG_DEF(JSMSG_DEBUG_CANT_SET_OPT_ENV, 1, JSEXN_REFERENCEERR, "can't set `{0}' in an optimized-out environment")
 MSG_DEF(JSMSG_DEBUG_INVISIBLE_COMPARTMENT, 0, JSEXN_TYPEERR, "object in compartment marked as invisible to Debugger")
 MSG_DEF(JSMSG_DEBUG_CENSUS_BREAKDOWN,  1, JSEXN_TYPEERR, "unrecognized 'by' value in takeCensus breakdown: {0}")
 MSG_DEF(JSMSG_DEBUG_PROMISE_NOT_RESOLVED, 0, JSEXN_TYPEERR, "Promise hasn't been resolved")
+MSG_DEF(JSMSG_DEBUG_PROMISE_NOT_FULFILLED, 0, JSEXN_TYPEERR, "Promise hasn't been fulfilled")
+MSG_DEF(JSMSG_DEBUG_PROMISE_NOT_REJECTED, 0, JSEXN_TYPEERR, "Promise hasn't been rejected")
 
 // Tracelogger
 MSG_DEF(JSMSG_TRACELOGGER_ENABLE_FAIL, 1, JSEXN_ERR, "enabling tracelogger failed: {0}")
 
 // Intl
 MSG_DEF(JSMSG_DATE_NOT_FINITE,         0, JSEXN_RANGEERR, "date value is not finite in DateTimeFormat.format()")
 MSG_DEF(JSMSG_INTERNAL_INTL_ERROR,     0, JSEXN_ERR, "internal error while computing Intl data")
 MSG_DEF(JSMSG_INTL_OBJECT_NOT_INITED,  3, JSEXN_TYPEERR, "Intl.{0}.prototype.{1} called on value that's not an object initialized as a {2}")
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -8662,34 +8662,34 @@ DebuggerObject::promiseStateGetter(JSCon
 DebuggerObject::promiseValueGetter(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_DEBUGOBJECT(cx, argc, vp, "get promiseValue", args, object);
 
     if (!DebuggerObject::requirePromise(cx, object))
         return false;
 
     if (object->promiseState() != JS::PromiseState::Fulfilled) {
-        args.rval().setUndefined();
-        return true;
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_PROMISE_NOT_FULFILLED);
+        return false;
     }
 
     return DebuggerObject::getPromiseValue(cx, object, args.rval());;
 }
 
 /* static */ bool
 DebuggerObject::promiseReasonGetter(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_DEBUGOBJECT(cx, argc, vp, "get promiseReason", args, object);
 
     if (!DebuggerObject::requirePromise(cx, object))
         return false;
 
     if (object->promiseState() != JS::PromiseState::Rejected) {
-        args.rval().setUndefined();
-        return true;
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_PROMISE_NOT_REJECTED);
+        return false;
     }
 
     return DebuggerObject::getPromiseReason(cx, object, args.rval());;
 }
 
 /* static */ bool
 DebuggerObject::promiseLifetimeGetter(JSContext* cx, unsigned argc, Value* vp)
 {