Repository: denoland/deno_core
Pull request #397 (2023-12-27 14:39:21)

fix(ops): support returning non-`anyhow` `Result` from fast sync op
```rs
// compiles
#[op2]
pub fn op_slow_anyhow_result(_: &mut v8::HandleScope) -> anyhow::Result<()> {
    Ok(())
}

// compiles
#[op2(fast)]
pub fn op_fast_anyhow_result() -> anyhow::Result<()> {
    Ok(())
}

// compiles
#[op2]
pub fn op_slow_std_io_result(_: &mut v8::HandleScope) -> std::io::Result<()> {
    Ok(())
}

// does not compile (fixed by this PR)
#[op2(fast)]
pub fn op_fast_std_io_result() -> std::io::Result<()> {
    Ok(())
}
```


CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/denoland/deno_core?pullRequest=397) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/denoland/deno_core?pullRequest=397) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/denoland/deno_core?pullRequest=397) it.</sub>
mmastrac: Looks good to me. Could you add a corresponding section to the ops/op2 README with a note that return values need to be `into anyhow::Error`?

Patch:
diff --git a/core/01_core.js b/core/01_core.js
index a8e1b7a..af6052e 100644
--- a/core/01_core.js
+++ b/core/01_core.js
@@ -9,6 +9,7 @@
     ArrayPrototypePush,
     Error,
     ErrorCaptureStackTrace,
+    FunctionPrototypeBind,
     MapPrototypeDelete,
     MapPrototypeGet,
     MapPrototypeHas,
@@ -18,6 +19,7 @@
     ObjectFreeze,
     ObjectFromEntries,
     ObjectKeys,
+    ObjectHasOwn,
     Promise,
     PromiseReject,
     PromiseResolve,
@@ -788,6 +790,78 @@
 
   const hostObjectBrand = SymbolFor("Deno.core.hostObject");
 
+  // A helper function that will bind our own console implementation
+  // with default implementation of Console from V8. This will cause
+  // console messages to be piped to inspector console.
+  //
+  // We are using `Deno.core.callConsole` binding to preserve proper stack
+  // frames in inspector console. This has to be done because V8 considers
+  // the last JS stack frame as gospel for the inspector. In our case we
+  // specifically want the latest user stack frame to be the one that matters
+  // though.
+  //
+  // Inspired by:
+  // https://github.com/nodejs/node/blob/1317252dfe8824fd9cfee125d2aaa94004db2f3b/lib/internal/util/inspector.js#L39-L61
+  function wrapConsole(customConsole, consoleFromV8) {
+    const callConsole = window.Deno.core.callConsole;
+
+    const keys = ObjectKeys(consoleFromV8);
+    for (let i = 0; i < keys.length; ++i) {
+      const key = keys[i];
+      if (ObjectHasOwn(customConsole, key)) {
+        customConsole[key] = FunctionPrototypeBind(
+          callConsole,
+          customConsole,
+          consoleFromV8[key],
+          customConsole[key],
+        );
+      } else {
+        // Add additional console APIs from the inspector
+        customConsole[key] = consoleFromV8[key];
+      }
+    }
+  }
+
+  // Minimal console implementation, that uses `Deno.core.print` under the hood.
+  // It's not fully fledged and is meant to make debugging slightly easier when working with
+  // only `deno_core` crate.
+  class CoreConsole {
+    #stringify = (arg) => {
+      if (
+        typeof arg === "string" || typeof arg === "boolean" ||
+        typeof arg === "number" || arg === null || arg === undefined
+      ) {
+        return arg;
+      }
+      return JSON.stringify(arg, undefined, 2);
+    };
+
+    log = (...args) => {
+      const stringifiedArgs = args.map(this.#stringify);
+      ops.op_print(`${stringifiedArgs.join(" ")}\n`, false);
+    };
+
+    debug = (...args) => {
+      const stringifiedArgs = args.map(this.#stringify);
+      ops.op_print(`${stringifiedArgs.join(" ")}\n`, false);
+    };
+
+    warn = (...args) => {
+      const stringifiedArgs = args.map(this.#stringify);
+      ops.op_print(`${stringifiedArgs.join(" ")}\n`, false);
+    };
+
+    error = (...args) => {
+      const stringifiedArgs = args.map(this.#stringify);
+      ops.op_print(`${stringifiedArgs.join(" ")}\n`, true);
+    };
+  }
+
+  const v8Console = globalThis.console;
+  const coreConsole = new CoreConsole();
+  globalThis.console = coreConsole;
+  wrapConsole(coreConsole, v8Console);
+
   const {
     op_is_any_array_buffer,
     op_is_arguments_object,
@@ -925,6 +999,8 @@
     build,
     setBuildInfo,
     currentUserCallSite,
+    wrapConsole,
+    v8Console,
   });
 
   const internals = {};
diff --git a/ops/op2/README.md b/ops/op2/README.md
index 42d5f30..4eac6df 100644
--- a/ops/op2/README.md
+++ b/ops/op2/README.md
@@ -15,6 +15,12 @@ string. At this time there is no way to avoid this copy, though the `op` code
 does attempt to avoid any allocations where possible by making use of a stack
 buffer.
 
+## Fallible `op`s
+
+An `op` function may be declared to return `Result` to indicate that the `op` is
+fallible. The error type must implement `Into<anyhow::Error>`. When the function
+returns `Err`, an exception is thrown.
+
 ## `async` calls
 
 Asynchronous calls are supported in two forms:
diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs
index ed4697a..4e531b7 100644
--- a/ops/op2/dispatch_fast.rs
+++ b/ops/op2/dispatch_fast.rs
@@ -334,6 +334,8 @@ pub(crate) fn generate_fast_result_early_exit(
     let #result = match #result {
       Ok(#result) => #result,
       Err(err) => {
+        let err = err.into();
+
         // FASTCALL FALLBACK: This is where we set the errors for the slow-call error pickup path. There
         // is no code running between this and the other FASTCALL FALLBACK comment, except some V8 code
         // required to perform the fallback process. This is why the below call is safe.


Test patch:
diff --git a/ops/op2/test_cases/sync/bool_result.out b/ops/op2/test_cases/sync/bool_result.out
index 97cbad4..cd73aa9 100644
--- a/ops/op2/test_cases/sync/bool_result.out
+++ b/ops/op2/test_cases/sync/bool_result.out
@@ -87,6 +87,7 @@ impl op_bool {
         let result = match result {
             Ok(result) => result,
             Err(err) => {
+                let err = err.into();
                 unsafe {
                     opctx.unsafely_set_last_error_for_ops_only(err);
                 }
diff --git a/ops/op2/test_cases/sync/result_external.out b/ops/op2/test_cases/sync/result_external.out
index df4fd35..6111daf 100644
--- a/ops/op2/test_cases/sync/result_external.out
+++ b/ops/op2/test_cases/sync/result_external.out
@@ -82,6 +82,7 @@ impl op_external_with_result {
         let result = match result {
             Ok(result) => result,
             Err(err) => {
+                let err = err.into();
                 unsafe {
                     opctx.unsafely_set_last_error_for_ops_only(err);
                 }
diff --git a/ops/op2/test_cases/sync/result_primitive.out b/ops/op2/test_cases/sync/result_primitive.out
index a6b3010..650d587 100644
--- a/ops/op2/test_cases/sync/result_primitive.out
+++ b/ops/op2/test_cases/sync/result_primitive.out
@@ -82,6 +82,7 @@ impl op_u32_with_result {
         let result = match result {
             Ok(result) => result,
             Err(err) => {
+                let err = err.into();
                 unsafe {
                     opctx.unsafely_set_last_error_for_ops_only(err);
                 }
diff --git a/ops/op2/test_cases/sync/result_void.out b/ops/op2/test_cases/sync/result_void.out
index b1015d3..a8a19a2 100644
--- a/ops/op2/test_cases/sync/result_void.out
+++ b/ops/op2/test_cases/sync/result_void.out
@@ -82,6 +82,7 @@ impl op_void_with_result {
         let result = match result {
             Ok(result) => result,
             Err(err) => {
+                let err = err.into();
                 unsafe {
                     opctx.unsafely_set_last_error_for_ops_only(err);
                 }
@@ -170,7 +171,7 @@ impl op_void_with_result {
         }
     }
     #[inline(always)]
-    pub fn call() -> Result<(), AnyError> {
+    pub fn call() -> std::io::Result<()> {
         Ok(())
     }
 }
diff --git a/ops/op2/test_cases/sync/result_void.rs b/ops/op2/test_cases/sync/result_void.rs
index ae0f793..6e51c91 100644
--- a/ops/op2/test_cases/sync/result_void.rs
+++ b/ops/op2/test_cases/sync/result_void.rs
@@ -2,9 +2,7 @@
 #![deny(warnings)]
 deno_ops_compile_test_runner::prelude!();
 
-use deno_core::error::AnyError;
-
 #[op2(fast)]
-pub fn op_void_with_result() -> Result<(), AnyError> {
+pub fn op_void_with_result() -> std::io::Result<()> {
     Ok(())
 }