Skip to content

promise: spec-compliant then/finally + SpeciesConstructor + error propagation #5767

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.

Sign up for GitHub

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
proggeramlug merged 2 commits into from
Jun 28, 2026
+385 −20
Merged

promise: spec-compliant then/finally + SpeciesConstructor + error propagation #5767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2274fa
promise: spec-compliant then/finally + SpeciesConstructor + error pro…
claude Jun 28, 2026
963e308
promise/then: guard arg_to_closure with is_callable_value; tighten fi…
claude Jun 28, 2026
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
24 changes: 16 additions & 8 deletions crates/perry-runtime/src/promise/spec_combinators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ pub enum CombinatorKind {

/// A spec `PromiseCapability` record: the constructed promise plus its
/// resolving functions, all as NaN-boxed JS values.
struct Capability {
promise: f64,
resolve: f64,
reject: f64,
pub(super) struct Capability {
pub(super) promise: f64,
pub(super) resolve: f64,
pub(super) reject: f64,
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -156,7 +156,7 @@ extern "C" fn capability_executor_fn(
/// `NewPromiseCapability(C)`. Throws (via `js_throw`) on the `IsConstructor`
/// failure, the executor "already called" failure (propagated from the
/// constructor), and the post-construct "resolve/reject not callable" failure.
fn new_promise_capability(c: f64) -> Capability {
pub(super) fn new_promise_capability(c: f64) -> Capability {
if !crate::object::js_value_is_constructor(c) {
throw_type_error("Promise.all called on non-constructor");
}
Expand Down Expand Up @@ -207,12 +207,16 @@ fn new_promise_capability(c: f64) -> Capability {
}
}

fn is_default_promise_constructor(c: f64) -> bool {
pub(super) fn is_default_promise_constructor(c: f64) -> bool {
let promise_ctor =
crate::object::js_get_global_this_builtin_value(b"Promise".as_ptr(), b"Promise".len());
!is_undef(promise_ctor) && promise_ctor.to_bits() == c.to_bits()
}
Comment on lines +210 to 214

@coderabbitai coderabbitai Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not identify %Promise% through mutable globalThis.Promise.

This helper reads the current global Promise field, so replacing globalThis.Promise can make the real intrinsic miss the fast path or make a user constructor look like the default. Use an immutable intrinsic Promise reference for this check, and share it with the SpeciesConstructor default path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/promise/spec_combinators.rs` around lines 210 - 214,
The default Promise check in is_default_promise_constructor is using the mutable
globalThis.Promise lookup, which can be changed by user code and break the fast
path. Update this helper to compare against an immutable intrinsic Promise
reference instead of js_get_global_this_builtin_value, and reuse that same
intrinsic reference in the SpeciesConstructor default-constructor path so both
checks stay consistent.


pub(super) fn is_callable_value(value: f64) -> bool {
is_callable(value)
}

// ---------------------------------------------------------------------------
// GetPromiseResolve(C) + Call/Invoke helpers
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -666,7 +670,9 @@ pub extern "C" fn js_promise_reject_spec(this_ctor: f64, reason: f64) -> f64 {
return js_nanbox_pointer(p as i64);
}
let cap = new_promise_capability(this_ctor);
let _ = call_with_this(cap.reject, undef(), &[reason]);
if let Err(thrown) = call_with_this(cap.reject, undef(), &[reason]) {
crate::exception::js_throw(thrown);
}
cap.promise
}

Expand Down Expand Up @@ -701,7 +707,9 @@ pub extern "C" fn js_promise_resolve_spec(this_ctor: f64, value: f64) -> f64 {
}
}
let cap = new_promise_capability(this_ctor);
let _ = call_with_this(cap.resolve, undef(), &[value]);
if let Err(thrown) = call_with_this(cap.resolve, undef(), &[value]) {
crate::exception::js_throw(thrown);
}
cap.promise
}

Expand Down
Loading
Loading