Pretty print promise objects (#2407)

Right now our promises print `{ }` on display. This PR improves a bit the display and ergonomics of promises in general. Now, promises will print...
- When pending: `Promise { <pending> }`
- When fulfilled: `Promise { "hi" }`
- When rejected: `Promise { <rejected> ReferenceError: x is not initialized }`

boa_engine/src/builtins/promise/mod.rs

@@ -58,17 +58,15 @@ macro_rules! if_abrupt_reject_promise {
 
 pub(crate) use if_abrupt_reject_promise;
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Trace, Finalize)]
-enum PromiseState {
+pub(crate) enum PromiseState {
     Pending,
-    Fulfilled,
+    Fulfilled(JsValue),
-    Rejected,
+    Rejected(JsValue),
 }
 
 #[derive(Debug, Clone, Trace, Finalize)]
 pub struct Promise {
-    promise_result: Option<JsValue>,
-    #[unsafe_ignore_trace]
     promise_state: PromiseState,
     promise_fulfill_reactions: Vec<ReactionRecord>,
     promise_reject_reactions: Vec<ReactionRecord>,
@@ -277,6 +275,11 @@ struct ResolvingFunctionsRecord {
 impl Promise {
     const LENGTH: usize = 1;
 
+    /// Gets the current state of the promise.
+    pub(crate) fn state(&self) -> &PromiseState {
+        &self.promise_state
+    }
+
     /// `Promise ( executor )`
     ///
     /// More information:
@@ -311,7 +314,6 @@ impl Promise {
         let promise = JsObject::from_proto_and_data(
             promise,
             ObjectData::promise(Self {
-                promise_result: None,
                 // 4. Set promise.[[PromiseState]] to pending.
                 promise_state: PromiseState::Pending,
                 // 5. Set promise.[[PromiseFulfillReactions]] to a new empty List.
@@ -1406,9 +1408,8 @@ impl Promise {
     /// [spec]: https://tc39.es/ecma262/#sec-fulfillpromise
     pub fn fulfill_promise(&mut self, value: &JsValue, context: &mut Context) -> JsResult<()> {
         // 1. Assert: The value of promise.[[PromiseState]] is pending.
-        assert_eq!(
+        assert!(
-            self.promise_state,
+            matches!(self.promise_state, PromiseState::Pending),
-            PromiseState::Pending,
             "promise was not pending"
         );
 
@@ -1419,17 +1420,15 @@ impl Promise {
         Self::trigger_promise_reactions(reactions, value, context);
         // reordering this statement does not affect the semantics
 
-        // 3. Set promise.[[PromiseResult]] to value.
-        self.promise_result = Some(value.clone());
-
         // 4. Set promise.[[PromiseFulfillReactions]] to undefined.
         self.promise_fulfill_reactions = Vec::new();
 
         // 5. Set promise.[[PromiseRejectReactions]] to undefined.
         self.promise_reject_reactions = Vec::new();
 
+        // 3. Set promise.[[PromiseResult]] to value.
         // 6. Set promise.[[PromiseState]] to fulfilled.
-        self.promise_state = PromiseState::Fulfilled;
+        self.promise_state = PromiseState::Fulfilled(value.clone());
 
         // 8. Return unused.
         Ok(())
@@ -1446,9 +1445,8 @@ impl Promise {
     /// [spec]: https://tc39.es/ecma262/#sec-rejectpromise
     pub fn reject_promise(&mut self, reason: &JsValue, context: &mut Context) {
         // 1. Assert: The value of promise.[[PromiseState]] is pending.
-        assert_eq!(
+        assert!(
-            self.promise_state,
+            matches!(self.promise_state, PromiseState::Pending),
-            PromiseState::Pending,
             "Expected promise.[[PromiseState]] to be pending"
         );
 
@@ -1459,17 +1457,15 @@ impl Promise {
         Self::trigger_promise_reactions(reactions, reason, context);
         // reordering this statement does not affect the semantics
 
-        // 3. Set promise.[[PromiseResult]] to reason.
-        self.promise_result = Some(reason.clone());
-
         // 4. Set promise.[[PromiseFulfillReactions]] to undefined.
         self.promise_fulfill_reactions = Vec::new();
 
         // 5. Set promise.[[PromiseRejectReactions]] to undefined.
         self.promise_reject_reactions = Vec::new();
 
+        // 3. Set promise.[[PromiseResult]] to reason.
         // 6. Set promise.[[PromiseState]] to rejected.
-        self.promise_state = PromiseState::Rejected;
+        self.promise_state = PromiseState::Rejected(reason.clone());
 
         // 7. If promise.[[PromiseIsHandled]] is false, perform HostPromiseRejectionTracker(promise, "reject").
         if !self.promise_is_handled {
@@ -1969,16 +1965,11 @@ impl Promise {
             }
 
             // 10. Else if promise.[[PromiseState]] is fulfilled, then
-            PromiseState::Fulfilled => {
+            //   a. Let value be promise.[[PromiseResult]].
-                //   a. Let value be promise.[[PromiseResult]].
+            PromiseState::Fulfilled(ref value) => {
-                let value = self
-                    .promise_result
-                    .clone()
-                    .expect("promise.[[PromiseResult]] cannot be empty");
-
                 //   b. Let fulfillJob be NewPromiseReactionJob(fulfillReaction, value).
                 let fulfill_job =
-                    PromiseJob::new_promise_reaction_job(fulfill_reaction, value, context);
+                    PromiseJob::new_promise_reaction_job(fulfill_reaction, value.clone(), context);
 
                 //   c. Perform HostEnqueuePromiseJob(fulfillJob.[[Job]], fulfillJob.[[Realm]]).
                 context.host_enqueue_promise_job(fulfill_job);
@@ -1986,13 +1977,8 @@ impl Promise {
 
             // 11. Else,
             //   a. Assert: The value of promise.[[PromiseState]] is rejected.
-            PromiseState::Rejected => {
+            //   b. Let reason be promise.[[PromiseResult]].
-                //   b. Let reason be promise.[[PromiseResult]].
+            PromiseState::Rejected(ref reason) => {
-                let reason = self
-                    .promise_result
-                    .clone()
-                    .expect("promise.[[PromiseResult]] cannot be empty");
-
                 //   c. If promise.[[PromiseIsHandled]] is false, perform HostPromiseRejectionTracker(promise, "handle").
                 if !self.promise_is_handled {
                     // HostPromiseRejectionTracker(promise, "handle")
@@ -2001,7 +1987,7 @@ impl Promise {
 
                 //   d. Let rejectJob be NewPromiseReactionJob(rejectReaction, reason).
                 let reject_job =
-                    PromiseJob::new_promise_reaction_job(reject_reaction, reason, context);
+                    PromiseJob::new_promise_reaction_job(reject_reaction, reason.clone(), context);
 
                 //   e. Perform HostEnqueuePromiseJob(rejectJob.[[Job]], rejectJob.[[Realm]]).
                 context.host_enqueue_promise_job(reject_job);

boa_engine/src/object/jsobject.rs

@@ -665,6 +665,19 @@ Cannot both specify accessors and a value or writable attribute",
         Ok(())
     }
 
+    #[inline]
+    pub(crate) fn get_property(&self, key: &PropertyKey) -> Option<PropertyDescriptor> {
+        let mut obj = Some(self.clone());
+
+        while let Some(o) = obj {
+            if let Some(v) = o.borrow().properties.get(key) {
+                return Some(v);
+            }
+            obj = o.borrow().prototype().clone();
+        }
+        None
+    }
+
     /// Helper function for property insertion.
     #[inline]
     #[track_caller]

boa_engine/src/object/mod.rs

@@ -192,9 +192,9 @@ pub enum ObjectKind {
     Arguments(Arguments),
     NativeObject(Box<dyn NativeObject>),
     IntegerIndexed(IntegerIndexed),
+    Promise(Promise),
     #[cfg(feature = "intl")]
     DateTimeFormat(Box<DateTimeFormat>),
-    Promise(Promise),
 }
 
 unsafe impl Trace for ObjectKind {

boa_engine/src/tests.rs

@@ -2645,7 +2645,7 @@ fn spread_getters_in_object() {
         var a = { x: 42 };
         var aWithXGetter = { ...a, ... { get x() { throw new Error('not thrown yet') } } };
     "#;
-    assert_eq!(&exec(scenario), "\"Error\": \"not thrown yet\"");
+    assert_eq!(&exec(scenario), "Error: not thrown yet");
 }
 
 #[test]

boa_engine/src/value/display.rs

@@ -1,4 +1,9 @@
-use crate::{js_string, object::ObjectKind, property::PropertyDescriptor};
+use std::borrow::Cow;
+
+use crate::{
+    builtins::promise::PromiseState, js_string, object::ObjectKind, property::PropertyDescriptor,
+    JsError, JsString,
+};
 
 use super::{fmt, Display, HashSet, JsValue, PropertyKey};
 
@@ -195,6 +200,54 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children
                         format!("Set({size})")
                     }
                 }
+                ObjectKind::Error(_) => {
+                    let name: Cow<'static, str> = v
+                        .get_property(&"name".into())
+                        .as_ref()
+                        .and_then(PropertyDescriptor::value)
+                        .map_or_else(
+                            || "<error>".into(),
+                            |v| {
+                                v.as_string()
+                                    .map_or_else(
+                                        || v.display().to_string(),
+                                        JsString::to_std_string_escaped,
+                                    )
+                                    .into()
+                            },
+                        );
+                    let message = v
+                        .get_property(&"message".into())
+                        .as_ref()
+                        .and_then(PropertyDescriptor::value)
+                        .map(|v| {
+                            v.as_string().map_or_else(
+                                || v.display().to_string(),
+                                JsString::to_std_string_escaped,
+                            )
+                        })
+                        .unwrap_or_default();
+                    if name.is_empty() {
+                        message
+                    } else if message.is_empty() {
+                        name.to_string()
+                    } else {
+                        format!("{name}: {message}")
+                    }
+                }
+                ObjectKind::Promise(ref promise) => {
+                    format!(
+                        "Promise {{ {} }}",
+                        match promise.state() {
+                            PromiseState::Pending => Cow::Borrowed("<pending>"),
+                            PromiseState::Fulfilled(val) => Cow::Owned(val.display().to_string()),
+                            PromiseState::Rejected(reason) => Cow::Owned(format!(
+                                "<rejected> {}",
+                                JsError::from_opaque(reason.clone())
+                            )),
+                        }
+                    )
+                }
                 _ => display_obj(x, print_internals),
             }
         }
@@ -255,26 +308,6 @@ pub(crate) fn display_obj(v: &JsValue, print_internals: bool) -> String {
     // in-memory address in this set
     let mut encounters = HashSet::new();
 
-    if let JsValue::Object(object) = v {
-        if object.borrow().is_error() {
-            let name = v
-                .get_property("name")
-                .as_ref()
-                .and_then(PropertyDescriptor::value)
-                .unwrap_or(&JsValue::Undefined)
-                .display()
-                .to_string();
-            let message = v
-                .get_property("message")
-                .as_ref()
-                .and_then(PropertyDescriptor::value)
-                .unwrap_or(&JsValue::Undefined)
-                .display()
-                .to_string();
-            return format!("{name}: {message}");
-        }
-    }
-
     display_obj_internal(v, &mut encounters, 4, print_internals)
 }