Merge pull request #401 from Shopify/js.return-error-when-module-errors

optionally return non-zero error code when function run sees error

Cargo.toml

@@ -3,6 +3,7 @@ members = [
   "tests/fixtures/exit_code",
   "tests/fixtures/log_truncation_function",
   "tests/fixtures/exports",
+  "tests/fixtures/noop",
 ]
 
 [package]

src/engine.rs

@@ -136,6 +136,7 @@ pub fn run(params: FunctionRunParams) -> Result<FunctionRunResult> {
     let memory_usage: u64;
     let instructions: u64;
     let mut error_logs: String = String::new();
+    let mut module_result: Result<(), anyhow::Error>;
     let profile_data: Option<String>;
 
     {
@@ -158,7 +159,6 @@ pub fn run(params: FunctionRunParams) -> Result<FunctionRunResult> {
 
         let func = instance.get_typed_func::<(), ()>(store.as_context_mut(), export)?;
 
-        let module_result;
         (module_result, profile_data) = if let Some(profile_opts) = profile_opts {
             let (result, profile_data) = wasmprof::ProfilerBuilder::new(&mut store)
                 .frequency(profile_opts.interval)
@@ -176,7 +176,7 @@ pub fn run(params: FunctionRunParams) -> Result<FunctionRunResult> {
         // modules may exit with a specific exit code, an exit code of 0 is considered success but is reported as
         // a GuestFault by wasmtime, so we need to map it to a success result. Any other exit code is considered
         // a failure.
-        let module_result =
+        module_result =
             module_result.or_else(|error| match error.downcast_ref::<wasi_common::I32Exit>() {
                 Some(I32Exit(0)) => Ok(()),
                 Some(I32Exit(code)) => Err(anyhow!("module exited with code: {}", code)),
@@ -188,7 +188,7 @@ pub fn run(params: FunctionRunParams) -> Result<FunctionRunResult> {
 
         match module_result {
             Ok(_) => {}
-            Err(e) => {
+            Err(ref e) => {
                 error_logs = e.to_string();
             }
         }
@@ -234,6 +234,7 @@ pub fn run(params: FunctionRunParams) -> Result<FunctionRunResult> {
         output,
         profile: profile_data,
         scale_factor,
+        success: module_result.is_ok(),
     };
 
     Ok(function_run_result)

src/function_run_result.rs

@@ -30,6 +30,7 @@ pub struct FunctionRunResult {
     pub profile: Option<String>,
     #[serde(skip)]
     pub scale_factor: f64,
+    pub success: bool,
 }
 
 const DEFAULT_INSTRUCTIONS_LIMIT: u64 = 11_000_000;
@@ -253,6 +254,7 @@ mod tests {
             })),
             profile: None,
             scale_factor: 1.0,
+            success: true,
         };
 
         let predicate = predicates::str::contains("Instructions: 1.001K")
@@ -284,6 +286,7 @@ mod tests {
             })),
             profile: None,
             scale_factor: 1.0,
+            success: true,
         };
 
         let predicate = predicates::str::contains("Instructions: 1")
@@ -311,6 +314,7 @@ mod tests {
             })),
             profile: None,
             scale_factor: 1.0,
+            success: true,
         };
 
         let predicate = predicates::str::contains("Instructions: 999")

src/main.rs

@@ -196,5 +196,9 @@ fn main() -> Result<()> {
         std::fs::write(profile_opts.unwrap().out, profile)?;
     }
 
-    Ok(())
+    if function_run_result.success {
+        Ok(())
+    } else {
+        anyhow::bail!("The Function execution failed. Review the logs for more information.")
+    }
 }

tests/fixtures/README.md

@@ -11,8 +11,8 @@ Example Functions used as test fixtures.
 
 **Rust examples:**
 ```
-cargo wasi build --profile=wasm -p exit_code -p exports -p log_truncation_function &&
-  cp target/wasm32-wasi/wasm/{exit_code.wasm,exports.wasm,log_truncation_function.wasm} tests/fixtures/build
+cargo wasi build --profile=wasm -p exit_code -p exports -p log_truncation_function -p noop &&
+  cp target/wasm32-wasi/wasm/{exit_code.wasm,exports.wasm,log_truncation_function.wasm,noop.wasm} tests/fixtures/build
 ```
 
 **JS examples:**
@@ -31,6 +31,11 @@ js_functions_javy_v1.wasm:
 javy build -C dynamic -C plugin=providers/shopify_functions_javy_v1.wasm -o tests/fixtures/build/js_functions_javy_v1.wasm tests/fixtures/js_function/src/functions.js
 ```
 
+js_function_that_throws.wasm:
+```
+javy build -C dynamic -C plugin=providers/javy_quickjs_provider_v3.wasm -o tests/fixtures/build/js_function_that_throws.wasm tests/fixtures/js_function_that_throws/src/functions.js
+```
+
 **`*.wat` examples:**
 ```
 find tests/fixtures -maxdepth 1 -type f -name "*.wat" \

tests/fixtures/js_function_that_throws/src/functions.js

@@ -0,0 +1,52 @@
+var __defProp = Object.defineProperty;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+
+// node_modules/javy/dist/fs/index.js
+var o = /* @__PURE__ */ ((r) => (r[r.Stdin = 0] = "Stdin", r[r.Stdout = 1] = "Stdout", r[r.Stderr = 2] = "Stderr", r))(o || {});
+function a(r) {
+  let e = new Uint8Array(1024), t = 0;
+  for (; ;) {
+    const i = Javy.IO.readSync(r, e.subarray(t));
+    if (i < 0)
+      throw Error("Error while reading from file descriptor");
+    if (i === 0)
+      return e.subarray(0, t + i);
+    if (t += i, t === e.length) {
+      const n = new Uint8Array(e.length * 2);
+      n.set(e), e = n;
+    }
+  }
+}
+function s(r, e) {
+  for (; e.length > 0;) {
+    const t = Javy.IO.writeSync(r, e);
+    if (t < 0)
+      throw Error("Error while writing to file descriptor");
+    e = e.subarray(t);
+  }
+}
+
+// extensions/volume-js/src/index.js
+var src_exports = {};
+__export(src_exports, {
+  default: () => src_default
+});
+var EMPTY_DISCOUNT = {
+  discountApplicationStrategy: "FIRST" /* First */,
+  discounts: []
+};
+var src_default = (input) => {
+  throw Error("A problem occurred.")
+};
+
+// node_modules/@shopify/shopify_function/index.ts
+var input_data = a(o.Stdin);
+var input_str = new TextDecoder("utf-8").decode(input_data);
+var input_obj = JSON.parse(input_str);
+var output_obj = src_exports?.default(input_obj);
+var output_str = JSON.stringify(output_obj);
+var output_data = new TextEncoder().encode(output_str);
+s(o.Stdout, output_data);

tests/fixtures/noop/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "noop"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+nanoserde = "0.1.37"

tests/fixtures/noop/src/main.rs

@@ -0,0 +1,9 @@
+use std::io;
+use std::io::Write;
+
+fn main() -> std::io::Result<()> {
+    let input_string = io::read_to_string(io::stdin())?;
+    std::io::stdout().write_all(input_string.as_bytes())?;
+    std::io::stdout().flush()?;
+    Ok(())
+}

tests/integration_tests.rs

@@ -15,9 +15,9 @@ mod tests {
     #[test]
     fn run() -> Result<(), Box<dyn std::error::Error>> {
         let mut cmd = Command::cargo_bin("function-runner")?;
-        let input_file = temp_input(json!({"exit_code": 0}))?;
+        let input_file = temp_input(json!({"count": 0}))?;
 
-        cmd.args(["--function", "tests/fixtures/build/exit_code.wasm"])
+        cmd.args(["--function", "tests/fixtures/build/noop.wasm"])
             .arg("--input")
             .arg(input_file.as_os_str());
         cmd.assert().success();
@@ -100,9 +100,9 @@ mod tests {
     #[test]
     fn run_json() -> Result<(), Box<dyn std::error::Error>> {
         let mut cmd = Command::cargo_bin("function-runner")?;
-        let input_file = temp_input(json!({"exit_code": 0}))?;
+        let input_file = temp_input(json!({"count": 0}))?;
 
-        cmd.args(["--function", "tests/fixtures/build/exit_code.wasm"])
+        cmd.args(["--function", "tests/fixtures/build/noop.wasm"])
             .arg("--json")
             .arg("--input")
             .arg(input_file.as_os_str());
@@ -146,8 +146,7 @@ mod tests {
     fn profile_writes_file() -> Result<(), Box<dyn std::error::Error>> {
         let (mut cmd, temp) = profile_base_cmd_in_temp_dir()?;
         cmd.arg("--profile").assert().success();
-        temp.child("exit_code.perf")
-            .assert(predicate::path::exists());
+        temp.child("noop.perf").assert(predicate::path::exists());
 
         Ok(())
     }
@@ -167,8 +166,7 @@ mod tests {
         cmd.args(["--profile-frequency", "80000"])
             .assert()
             .success();
-        temp.child("exit_code.perf")
-            .assert(predicate::path::exists());
+        temp.child("noop.perf").assert(predicate::path::exists());
 
         Ok(())
     }
@@ -183,10 +181,13 @@ mod tests {
             .arg(input_file.as_os_str());
 
         cmd.assert()
-            .success()
+            .failure()
             .stdout(contains("Key not found code"))
             .stdout(contains("Invalid Output"))
             .stdout(contains("JSON Error"))
+            .stderr(contains(
+                "Error: The Function execution failed. Review the logs for more information.",
+            ))
             .stderr(contains(""));
 
         Ok(())
@@ -221,17 +222,36 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn failing_function_returns_non_zero_exit_code_for_module_errors(
+    ) -> Result<(), Box<dyn std::error::Error>> {
+        let mut cmd = Command::cargo_bin("function-runner")?;
+        let input_file = temp_input(json!({}))?;
+        cmd.args([
+            "--function",
+            "tests/fixtures/build/js_function_that_throws.wasm",
+        ])
+        .arg("--input")
+        .arg(input_file.as_os_str());
+
+        cmd.assert().failure().stderr(contains(
+            "The Function execution failed. Review the logs for more information.",
+        ));
+
+        Ok(())
+    }
+
     fn profile_base_cmd_in_temp_dir(
     ) -> Result<(Command, assert_fs::TempDir), Box<dyn std::error::Error>> {
         let mut cmd = Command::cargo_bin("function-runner")?;
         let cwd = std::env::current_dir()?;
         let temp = assert_fs::TempDir::new()?;
         let input_file = temp.child("input.json");
-        input_file.write_str(json!({"exit_code": 0}).to_string().as_str())?;
+        input_file.write_str(json!({"count": 0}).to_string().as_str())?;
 
         cmd.current_dir(temp.path())
             .arg("--function")
-            .arg(cwd.join("tests/fixtures/build/exit_code.wasm"))
+            .arg(cwd.join("tests/fixtures/build/noop.wasm"))
             .arg("--input")
             .arg(input_file.as_os_str());
 
@@ -257,7 +277,7 @@ mod tests {
             ]
         }}))?;
 
-        cmd.args(["--function", "tests/fixtures/build/exit_code.wasm"])
+        cmd.args(["--function", "tests/fixtures/build/noop.wasm"])
             .arg("--input")
             .arg(input_file.as_os_str());
         cmd.assert().success();
@@ -281,7 +301,7 @@ mod tests {
             ]
         }}))?;
 
-        cmd.args(["--function", "tests/fixtures/build/exit_code.wasm"])
+        cmd.args(["--function", "tests/fixtures/build/noop.wasm"])
             .arg("--input")
             .arg(input_file.as_os_str())
             .arg("--schema-path")
@@ -309,7 +329,7 @@ mod tests {
         });
         let input_file = temp_input(json_data)?;
 
-        cmd.args(["--function", "tests/fixtures/build/exit_code.wasm"])
+        cmd.args(["--function", "tests/fixtures/build/noop.wasm"])
             .arg("--input")
             .arg(input_file.as_os_str())
             .arg("--schema-path")