Repository: denoland/deno_core
Pull request #54 (2023-07-19 15:04:09)

feat(ops): op2 support for Result<impl Future<...>> 
Supports `Result<impl Future<...>>`-style async ops like so:

```rust
  #[op2(async, core)]
  pub fn op_async_result_impl(
    mode: u8,
  ) -> Result<impl Future<Output = Result<(), Error>>, Error> {
    if mode == 0 {
      return Err(generic_error("early exit"));
    }
    return Ok(async move {
      if mode == 1 {
        return Err(generic_error("early async exit"));
      }
      tokio::time::sleep(Duration::from_millis(500)).await;
      if mode == 2 {
        return Err(generic_error("late async exit"));
      }
      return Ok(());
    });
  }
```


Patch:
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index 2d6797a..e18512b 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -550,6 +550,7 @@ mod tests {
       op_async_sleep,
       op_async_sleep_impl,
       op_async_sleep_error,
+      op_async_result_impl,
       op_async_state_rc,
       op_async_buffer_impl,
     ],
@@ -593,6 +594,9 @@ mod tests {
                 op_test_fail();
               }}
             }}
+            function assertErrorContains(e, s) {{
+              assert(String(e).indexOf(s) != -1)
+            }}
             function log(s) {{
               op_test_print_debug(String(s))
             }}
@@ -647,6 +651,9 @@ mod tests {
                 op_test_fail();
               }}
             }}
+            function assertErrorContains(e, s) {{
+              assert(String(e).indexOf(s) != -1)
+            }}
             function log(s) {{
               op_test_print_debug(String(s))
             }}
@@ -1154,7 +1161,7 @@ mod tests {
     run_test2(
       1,
       "op_test_serde_v8",
-      "try { op_test_serde_v8({}); assert(false) } catch (e) { assert(String(e).indexOf('missing field') != -1) }",
+      "try { op_test_serde_v8({}); assert(false) } catch (e) { assertErrorContains(e, 'missing field') }",
     )?;
     Ok(())
   }
@@ -1414,6 +1421,47 @@ mod tests {
     Ok(())
   }
 
+  /// Test exits from the three possible routes -- before future, future immediate,
+  /// future polled failed, future polled success.
+  #[op2(async, core)]
+  pub fn op_async_result_impl(
+    mode: u8,
+  ) -> Result<impl Future<Output = Result<(), Error>>, Error> {
+    if mode == 0 {
+      return Err(generic_error("early exit"));
+    }
+    Ok(async move {
+      if mode == 1 {
+        return Err(generic_error("early async exit"));
+      }
+      tokio::time::sleep(Duration::from_millis(500)).await;
+      if mode == 2 {
+        return Err(generic_error("late async exit"));
+      }
+      Ok(())
+    })
+  }
+
+  #[tokio::test]
+  pub async fn test_op_async_result_impl(
+  ) -> Result<(), Box<dyn std::error::Error>> {
+    for (n, msg) in [
+      (0, "early exit"),
+      (1, "early async exit"),
+      (2, "late async exit"),
+    ] {
+      run_async_test(
+        5,
+        "op_async_result_impl",
+        &format!("try {{ await op_async_result_impl({n}); assert(false) }} catch (e) {{ assertErrorContains(e, '{msg}') }}"),
+      )
+      .await?;
+    }
+    run_async_test(5, "op_async_result_impl", "await op_async_result_impl(3);")
+      .await?;
+    Ok(())
+  }
+
   #[op2(async, core)]
   pub async fn op_async_state_rc(
     state: Rc<RefCell<OpState>>,
diff --git a/ops/op2/dispatch_async.rs b/ops/op2/dispatch_async.rs
index ca05cfc..27d63b4 100644
--- a/ops/op2/dispatch_async.rs
+++ b/ops/op2/dispatch_async.rs
@@ -7,6 +7,7 @@ use super::dispatch_slow::from_arg;
 use super::dispatch_slow::return_value_infallible;
 use super::dispatch_slow::return_value_result;
 use super::dispatch_slow::return_value_v8_value;
+use super::dispatch_slow::throw_exception;
 use super::dispatch_slow::with_fn_args;
 use super::dispatch_slow::with_opctx;
 use super::dispatch_slow::with_opstate;
@@ -82,26 +83,31 @@ pub(crate) fn generate_dispatch_async(
 
   output.extend(deferred);
   output.extend(call(generator_state)?);
-  let result = &generator_state.result;
-  let opctx = &generator_state.opctx;
-  let scope = &generator_state.scope;
-  let fn_args = &generator_state.fn_args;
-  let promise_id = &generator_state.promise_id;
-  let deno_core = &generator_state.deno_core;
 
   if matches!(
     signature.ret_val,
     RetVal::ResultFuture(_) | RetVal::ResultFutureResult(_)
   ) {
+    let exception = throw_exception(generator_state)?;
+    let result = &generator_state.result;
     output.extend(quote! {
-      todo!("result of future currently not implemented");
-      let Ok(#result) = #result else {
-        // Handle eager error -- this will leave only a Future<R> or Future<Result<R>>
-        // TODO(mmastrac): This is unimplemented
+      let #result = match #result {
+        Ok(#result) => #result,
+        Err(err) => {
+          // Handle eager error -- this will leave only a Future<R> or Future<Result<R>>
+          #exception
+        }
       };
     });
   }
 
+  let result = &generator_state.result;
+  let opctx = &generator_state.opctx;
+  let scope = &generator_state.scope;
+  let fn_args = &generator_state.fn_args;
+  let promise_id = &generator_state.promise_id;
+  let deno_core = &generator_state.deno_core;
+
   output.extend(quote! {
     // TODO(mmastrac): We are extending the eager polling behaviour temporarily to op2, but I would like to get
     // rid of it as soon as we can
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 68616a6..9b55098 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -658,7 +658,7 @@ pub fn return_value_result(
 }
 
 /// Generates code to throw an exception, adding required additional dependencies as needed.
-fn throw_exception(
+pub(crate) fn throw_exception(
   generator_state: &mut GeneratorState,
 ) -> Result<TokenStream, V8MappingError> {
   let maybe_scope = if generator_state.needs_scope {


Test patch:
diff --git a/ops/op2/test_cases/async/async_result_impl.out b/ops/op2/test_cases/async/async_result_impl.out
new file mode 100644
index 0000000..300ff64
--- /dev/null
+++ b/ops/op2/test_cases/async/async_result_impl.out
@@ -0,0 +1,84 @@
+#[allow(non_camel_case_types)]
+pub struct op_async_result_impl {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_async_result_impl {
+    const NAME: &'static str = stringify!(op_async_result_impl);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl::new_internal(
+        stringify!(op_async_result_impl),
+        true,
+        false,
+        false,
+        2usize as u8,
+        Self::v8_fn_ptr as _,
+        None,
+    );
+}
+impl op_async_result_impl {
+    pub const fn name() -> &'static str {
+        stringify!(op_async_result_impl)
+    }
+    #[deprecated(note = "Use the const op::DECL instead")]
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        <Self as deno_core::_ops::Op>::DECL
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
+        let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+            &*info
+        });
+        let opctx = unsafe {
+            &*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data()).value()
+                as *const deno_core::_ops::OpCtx)
+        };
+        let arg0 = args.get(1usize as i32);
+        let arg0 = deno_core::_ops::to_i32(&arg0) as _;
+        let result = Self::call(arg0);
+        let result = match result {
+            Ok(result) => result,
+            Err(err) => {
+                let err = err.into();
+                let opstate = ::std::cell::RefCell::borrow(&*opctx.state);
+                let exception = deno_core::error::to_v8_error(
+                    &mut scope,
+                    opstate.get_error_class_fn,
+                    &err,
+                );
+                scope.throw_exception(exception);
+                return;
+            }
+        };
+        let promise_id = deno_core::_ops::to_i32(&args.get(0));
+        if let Some(result)
+            = deno_core::_ops::map_async_op_fallible(
+                opctx,
+                promise_id,
+                result,
+                |scope, result| { Ok(deno_core::v8::Integer::new(scope, result).into()) },
+            ) {
+            match result {
+                Ok(result) => {
+                    rv.set_int32(result as i32);
+                }
+                Err(err) => {
+                    let err = err.into();
+                    let opstate = ::std::cell::RefCell::borrow(&*opctx.state);
+                    let exception = deno_core::error::to_v8_error(
+                        &mut scope,
+                        opstate.get_error_class_fn,
+                        &err,
+                    );
+                    scope.throw_exception(exception);
+                    return;
+                }
+            };
+        }
+    }
+    #[inline(always)]
+    pub fn call(x: i32) -> Result<impl Future<Output = std::io::Result<i32>>, AnyError> {
+        Ok(async move { Ok(x) })
+    }
+}
diff --git a/ops/op2/test_cases/async/async_result_impl.rs b/ops/op2/test_cases/async/async_result_impl.rs
new file mode 100644
index 0000000..71588ba
--- /dev/null
+++ b/ops/op2/test_cases/async/async_result_impl.rs
@@ -0,0 +1,13 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+#![deny(warnings)]
+deno_ops_compile_test_runner::prelude!();
+
+use std::future::Future;
+use deno_core::error::AnyError;
+
+#[op2(async)]
+pub fn op_async_result_impl(x: i32) -> Result<impl Future<Output = std::io::Result<i32>>, AnyError> {
+    Ok(async move {
+        Ok(x)
+    })
+}
diff --git a/ops/op2/test_cases/compiler_pass/async.rs b/ops/op2/test_cases/compiler_pass/async.rs
index fcacd6c..fab40ca 100644
--- a/ops/op2/test_cases/compiler_pass/async.rs
+++ b/ops/op2/test_cases/compiler_pass/async.rs
@@ -4,6 +4,8 @@ deno_ops_compile_test_runner::prelude!();
 
 use std::rc::Rc;
 use std::cell::RefCell;
+use std::future::Future;
+use deno_core::error::AnyError;
 use deno_core::OpState;
 
 // Collect a few examples that we'll smoke test when not running on the CI.
@@ -21,6 +23,20 @@ pub async fn op_async3(x: i32) -> std::io::Result<i32> {
     Ok(x)
 }
 
+#[op2(async)]
+pub fn op_async4(x: i32) -> Result<impl Future<Output = i32>, AnyError> {
+    Ok(async move {
+        x
+    })
+}
+
+#[op2(async)]
+pub fn op_async5(x: i32) -> Result<impl Future<Output = std::io::Result<i32>>, AnyError> {
+    Ok(async move {
+        Ok(x)
+    })
+}
+
 #[op2(async)]
 pub async fn op_async_opstate(state: Rc<RefCell<OpState>>) -> std::io::Result<i32> {
     Ok(*state.borrow().borrow::<i32>())