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

[Merged by Bors] - Pretty print promise objects #2407

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 23 additions & 37 deletions boa_engine/src/builtins/promise/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,15 @@ macro_rules! if_abrupt_reject_promise {

pub(crate) use if_abrupt_reject_promise;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum PromiseState {
#[derive(Debug, Clone, Trace, Finalize)]
pub(crate) enum PromiseState {
Pending,
Fulfilled,
Rejected,
Fulfilled(JsValue),
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>,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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!(
self.promise_state,
PromiseState::Pending,
assert!(
matches!(self.promise_state, PromiseState::Pending),
"promise was not pending"
);

Expand All @@ -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(())
Expand All @@ -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!(
self.promise_state,
PromiseState::Pending,
assert!(
matches!(self.promise_state, PromiseState::Pending),
"Expected promise.[[PromiseState]] to be pending"
);

Expand All @@ -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 {
Expand Down Expand Up @@ -1969,30 +1965,20 @@ impl Promise {
}

// 10. Else if promise.[[PromiseState]] is fulfilled, then
PromiseState::Fulfilled => {
// a. Let value be promise.[[PromiseResult]].
let value = self
.promise_result
.clone()
.expect("promise.[[PromiseResult]] cannot be empty");

// a. Let value be promise.[[PromiseResult]].
PromiseState::Fulfilled(ref value) => {
// 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);
}

// 11. Else,
// a. Assert: The value of promise.[[PromiseState]] is rejected.
PromiseState::Rejected => {
// b. Let reason be promise.[[PromiseResult]].
let reason = self
.promise_result
.clone()
.expect("promise.[[PromiseResult]] cannot be empty");

// b. Let reason be promise.[[PromiseResult]].
PromiseState::Rejected(ref reason) => {
// c. If promise.[[PromiseIsHandled]] is false, perform HostPromiseRejectionTracker(promise, "handle").
if !self.promise_is_handled {
// HostPromiseRejectionTracker(promise, "handle")
Expand All @@ -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);
Expand Down
13 changes: 13 additions & 0 deletions boa_engine/src/object/jsobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
60 changes: 39 additions & 21 deletions boa_engine/src/value/display.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::{js_string, object::ObjectKind, property::PropertyDescriptor};
use std::borrow::Cow;

use crate::{builtins::promise::PromiseState, js_string, object::ObjectKind, JsError, JsString};

use super::{fmt, Display, HashSet, JsValue, PropertyKey};

Expand Down Expand Up @@ -195,6 +197,42 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children
format!("Set({size})")
}
}
ObjectKind::Error(_) => {
let name = v
.get_property(&"name".into())
.and_then(|desc| {
desc.value()
.and_then(JsValue::as_string)
.map(JsString::to_std_string_escaped)
})
.unwrap_or_else(|| "<error>".into());
let message = v
.get_property(&"message".into())
.and_then(|desc| {
desc.value()
.and_then(JsValue::as_string)
.map(JsString::to_std_string_escaped)
})
.unwrap_or_default();
if message.is_empty() {
name
} 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),
}
}
Expand Down Expand Up @@ -255,26 +293,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)
}

Expand Down