fix(vm): restore async-generator invariants and PromiseResolve abrupt handling (#706, #707, #708, #709)

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs

@@ -37,25 +37,25 @@ impl AsyncGenerator<'_> {
         self.get(agent).executable.unwrap().bind(gc)
     }
 
-    /// Returns true if the state of the AsyncGenerator is DRAINING-QUEUE or
-    /// EXECUTING.
-    ///
-    /// > NOTE: In our implementation, EXECUTING is split into an extra
-    /// > EXECUTING-AWAIT state. This also checks for that.
-    pub(crate) fn is_active(self, agent: &Agent) -> bool {
+    pub(crate) fn is_draining_queue(self, agent: &Agent) -> bool {
         self.get(agent)
             .async_generator_state
             .as_ref()
             .unwrap()
-            .is_active()
+            .is_draining_queue()
     }
 
-    pub(crate) fn is_draining_queue(self, agent: &Agent) -> bool {
+    /// Returns true if the state of the AsyncGenerator is DRAINING-QUEUE or
+    /// EXECUTING.
+    ///
+    /// NOTE: In our implementation, EXECUTING is split into an extra
+    /// EXECUTING-AWAIT state. This also checks for that.
+    pub(crate) fn is_active(self, agent: &Agent) -> bool {
         self.get(agent)
             .async_generator_state
             .as_ref()
             .unwrap()
-            .is_draining_queue()
+            .is_active()
     }
 
     pub(crate) fn is_executing(self, agent: &Agent) -> bool {
@@ -200,17 +200,18 @@ impl AsyncGenerator<'_> {
         gc: NoGcScope<'gc, '_>,
     ) -> (SuspendedVm, ExecutionContext, Executable<'gc>) {
         let async_generator_state = &mut self.get_mut(agent).async_generator_state;
-        let (vm, execution_context, queue) = match async_generator_state.take() {
-            Some(AsyncGeneratorState::SuspendedStart {
+        let state = async_generator_state.take().unwrap();
+        let (vm, execution_context, queue) = match state {
+            AsyncGeneratorState::SuspendedStart {
                 vm,
                 execution_context,
                 queue,
-            }) => (vm, execution_context, queue),
-            Some(AsyncGeneratorState::SuspendedYield {
+            } => (vm, execution_context, queue),
+            AsyncGeneratorState::SuspendedYield {
                 vm,
                 execution_context,
                 queue,
-            }) => (vm, execution_context, queue),
+            } => (vm, execution_context, queue),
             _ => unreachable!(),
         };
         async_generator_state.replace(AsyncGeneratorState::Executing(queue));
@@ -258,6 +259,11 @@ impl AsyncGenerator<'_> {
         // 1. Assert: generator.[[AsyncGeneratorState]] is either suspended-start or suspended-yield.
         let state = self.get_mut(agent).async_generator_state.take().unwrap();
         let (vm, execution_context, queue, kind) = match state {
+            AsyncGeneratorState::SuspendedStart {
+                vm,
+                execution_context,
+                queue,
+            } => (vm, execution_context, queue, AsyncGeneratorAwaitKind::Await),
             AsyncGeneratorState::SuspendedYield {
                 vm,
                 execution_context,

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

@@ -5,8 +5,8 @@
 use crate::{
     ecmascript::{
         Agent, ECMAScriptFunction, ExceptionType, JsError, JsResult, Promise, PromiseCapability,
-        PromiseReactionHandler, Realm, Value, create_iter_result_object, inner_promise_then,
-        unwrap_try,
+        PromiseReactionHandler, PromiseReactionType, Realm, Value, create_iter_result_object,
+        inner_promise_then, unwrap_try,
     },
     engine::{Bindable, ExecutionResult, GcScope, NoGcScope, Scopable, Scoped, SuspendedVm},
     heap::ArenaAccessMut,
@@ -257,14 +257,19 @@ fn async_generator_perform_await(
     let execution_context = agent.pop_execution_context().unwrap();
     let generator = scoped_generator.get(agent).bind(gc.nogc());
     generator.transition_to_awaiting(agent, vm, kind, execution_context);
+    let generator = generator.unbind();
     // 8. Remove asyncContext from the execution context stack and
     //    restore the execution context that is at the top of the
     //    execution context stack as the running execution context.
-    let handler = PromiseReactionHandler::AsyncGenerator(generator.unbind());
+    let handler = PromiseReactionHandler::AsyncGenerator(generator);
     // 2. Let promise be ? PromiseResolve(%Promise%, value).
-    let promise = Promise::resolve(agent, awaited_value, gc.reborrow())
-        .unbind()
-        .bind(gc.nogc());
+    let promise = match Promise::try_resolve(agent, awaited_value, gc.reborrow()) {
+        Ok(promise) => promise.unbind().bind(gc.nogc()),
+        Err(err) => {
+            generator.resume_await(agent, PromiseReactionType::Reject, err.value().unbind(), gc);
+            return;
+        }
+    };
 
     // 7. Perform PerformPromiseThen(promise, onFulfilled, onRejected).
     inner_promise_then(agent, promise, handler, handler, None, gc.nogc());
@@ -404,16 +409,34 @@ pub(crate) fn async_generator_await_return(
     let AsyncGeneratorRequestCompletion::Return(value) = completion else {
         unreachable!()
     };
+    let return_value = value.unbind().bind(gc.nogc());
     // 7. Let promiseCompletion be Completion(PromiseResolve(%Promise%, completion.[[Value]])).
-    // 8. If promiseCompletion is an abrupt completion, then
-    //         a. Perform AsyncGeneratorCompleteStep(generator, promiseCompletion, true).
-    //         b. Perform AsyncGeneratorDrainQueue(generator).
-    //         c. Return unused.
+    let promise_completion = Promise::try_resolve(agent, return_value.unbind(), gc.reborrow())
+        .map(|promise| promise.unbind())
+        .map_err(|err| err.unbind());
+    let promise = match promise_completion {
+        // 8. If promiseCompletion is an abrupt completion, then
+        //         a. Perform AsyncGeneratorCompleteStep(generator, promiseCompletion, true).
+        //         b. Perform AsyncGeneratorDrainQueue(generator).
+        //         c. Return unused.
+        Err(err) => {
+            let generator = scoped_generator.get(agent).bind(gc.nogc());
+            async_generator_complete_step(
+                agent,
+                generator.unbind(),
+                AsyncGeneratorRequestCompletion::Err(err.unbind()),
+                true,
+                None,
+                gc.nogc(),
+            );
+            async_generator_drain_queue(agent, scoped_generator, gc);
+            return;
+        }
+        Ok(promise) => promise.unbind().bind(gc.nogc()),
+    };
+
     // 9. Assert: promiseCompletion is a normal completion.
     // 10. Let promise be promiseCompletion.[[Value]].
-    let promise = Promise::resolve(agent, value.unbind(), gc.reborrow())
-        .unbind()
-        .bind(gc.nogc());
     // 11. ... onFulfilled ...
     // 12. Let onFulfilled be CreateBuiltinFunction(fulfilledClosure, 1, "", « »).
     // 13. ... onRejected ...
@@ -498,7 +521,9 @@ fn async_generator_drain_queue(
     let data = generator.get_mut(agent);
     // Assert: generator.[[AsyncGeneratorState]] is draining-queue.
     // 2. Let queue be generator.[[AsyncGeneratorQueue]].
-    let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state else {
+    let AsyncGeneratorState::DrainingQueue(queue) =
+        &mut data.async_generator_state.as_mut().unwrap()
+    else {
         unreachable!()
     };
     // 3. If queue is empty, then
@@ -536,7 +561,8 @@ fn async_generator_drain_queue(
             async_generator_complete_step(agent, generator, completion, true, None, gc.nogc());
             let data = generator.get_mut(agent);
             // iii. If queue is empty, then
-            let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state
+            let AsyncGeneratorState::DrainingQueue(queue) =
+                &mut data.async_generator_state.as_mut().unwrap()
             else {
                 unreachable!()
             };

nova_vm/src/ecmascript/builtins/global_object.rs

@@ -20,7 +20,10 @@ use crate::{
         script_var_scoped_declarations, to_int32, to_int32_number, to_number, to_number_primitive,
         to_string,
     },
-    engine::{Bindable, Executable, GcScope, NoGcScope, Scopable, Vm, string_literal_to_wtf8},
+    engine::{
+        Bindable, Executable, ExecutionResult, GcScope, NoGcScope, Scopable, Vm,
+        string_literal_to_wtf8,
+    },
     heap::{ArenaAccess, HeapIndexHandle, IntrinsicFunctionIndexes},
     ndt,
 };
@@ -391,7 +394,17 @@ pub(crate) fn perform_eval<'gc>(
                 // a. Set result to Completion(Evaluation of body).
                 // 30. If result is a normal completion and result.[[Value]] is empty, then
                 // a. Set result to NormalCompletion(undefined).
-                let result = Vm::execute(agent, exe.clone(), None, gc).into_js_result();
+                let result = match Vm::execute(agent, exe.clone(), None, gc.reborrow()) {
+                    ExecutionResult::Return(value) => Ok(value.unbind()),
+                    ExecutionResult::Throw(err) => Err(err.unbind()),
+                    ExecutionResult::Await { .. } | ExecutionResult::Yield { .. } => Err(agent
+                        .throw_exception_with_static_message(
+                            ExceptionType::SyntaxError,
+                            "Invalid eval source text: unexpected await or yield in script.",
+                            gc.nogc(),
+                        )
+                        .unbind()),
+                };
                 // SAFETY: No one can access the bytecode anymore.
                 unsafe { exe.take(agent).try_drop(agent) };
                 result

nova_vm/src/ecmascript/builtins/promise.rs

@@ -8,8 +8,8 @@ pub(crate) use data::*;
 
 use crate::{
     ecmascript::{
-        Agent, InternalMethods, InternalSlots, JsError, JsResult, OrdinaryObject,
-        PromiseCapability, ProtoIntrinsics, Value, object_handle,
+        Agent, BUILTIN_STRING_MEMORY, InternalMethods, InternalSlots, JsError, JsResult,
+        OrdinaryObject, PromiseCapability, ProtoIntrinsics, Value, get, object_handle,
     },
     engine::{Bindable, GcScope, NoGcScope, Scopable},
     heap::{
@@ -25,6 +25,40 @@ object_handle!(Promise);
 arena_vec_access!(Promise, 'a, PromiseHeapData, promises);
 
 impl<'a> Promise<'a> {
+    ///### [27.2.4.7.1 PromiseResolve ( C, x )](https://tc39.es/ecma262/#sec-promise-resolve)
+    ///
+    /// This variant implements abrupt-completion behavior for internal callers
+    /// that must handle `? PromiseResolve(%Promise%, x)`.
+    pub fn try_resolve(agent: &mut Agent, x: Value, mut gc: GcScope<'a, '_>) -> JsResult<'a, Self> {
+        // 1. If IsPromise(x) is true, then
+        if let Value::Promise(promise) = x {
+            // a. Let xConstructor be ? Get(x, "constructor").
+            let x_constructor = match get(
+                agent,
+                promise,
+                BUILTIN_STRING_MEMORY.constructor.into(),
+                gc.reborrow(),
+            ) {
+                Ok(value) => value.unbind().bind(gc.nogc()),
+                Err(err) => return Err(err.unbind().bind(gc.into_nogc())),
+            };
+            // b. If SameValue(xConstructor, C) is true, return x.
+            // NOTE: Ignoring subclasses.
+            if x_constructor == agent.current_realm_record().intrinsics().promise().into() {
+                return Ok(promise.bind(gc.into_nogc()));
+            }
+        }
+        // 2. Let promiseCapability be ? NewPromiseCapability(C).
+        let promise_capability = PromiseCapability::new(agent, gc.nogc());
+        let promise = promise_capability.promise().scope(agent, gc.nogc());
+        // 3. Perform ? Call(promiseCapability.[[Resolve]], undefined, « x »).
+        // NOTE: Promise capability resolve never throws in Nova's internal model.
+        promise_capability.unbind().resolve(agent, x, gc.reborrow());
+        // 4. Return promiseCapability.[[Promise]].
+        // SAFETY: Not shared.
+        Ok(unsafe { promise.take(agent) }.bind(gc.into_nogc()))
+    }
+
     /// Create a new resolved Promise.
     pub(crate) fn new_resolved(agent: &mut Agent, value: Value<'a>) -> Self {
         agent.heap.create(PromiseHeapData {

tests/expectations.json

@@ -226,9 +226,6 @@
   "built-ins/AsyncGeneratorFunction/proto-from-ctor-realm-prototype.js": "FAIL",
   "built-ins/AsyncGeneratorFunction/proto-from-ctor-realm.js": "FAIL",
   "built-ins/AsyncGeneratorFunction/prototype/constructor.js": "FAIL",
-  "built-ins/AsyncGeneratorPrototype/return/return-state-completed-broken-promise.js": "FAIL",
-  "built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js": "FAIL",
-  "built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js": "FAIL",
   "built-ins/AsyncIteratorPrototype/Symbol.asyncDispose/invokes-return.js": "FAIL",
   "built-ins/AsyncIteratorPrototype/Symbol.asyncDispose/is-function.js": "FAIL",
   "built-ins/AsyncIteratorPrototype/Symbol.asyncDispose/length.js": "FAIL",
@@ -7109,7 +7106,6 @@
   "staging/sm/Array/unscopables.js": "FAIL",
   "staging/sm/Array/values.js": "FAIL",
   "staging/sm/ArrayBuffer/slice-species.js": "FAIL",
-  "staging/sm/AsyncGenerators/for-await-of-error.js": "CRASH",
   "staging/sm/BigInt/Number-conversion-rounding.js": "FAIL",
   "staging/sm/Date/dst-offset-caching-1-of-8.js": "TIMEOUT",
   "staging/sm/Date/dst-offset-caching-2-of-8.js": "TIMEOUT",
@@ -7381,7 +7377,6 @@
   "staging/sm/TypedArray/toLocaleString.js": "FAIL",
   "staging/sm/TypedArray/toString.js": "FAIL",
   "staging/sm/TypedArray/values.js": "FAIL",
-  "staging/sm/async-functions/await-error.js": "CRASH",
   "staging/sm/async-functions/await-in-arrow-parameters.js": "FAIL",
   "staging/sm/async-functions/await-in-parameters-of-async-func.js": "FAIL",
   "staging/sm/async-functions/toString.js": "FAIL",