Skip to content
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.

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

Handle runtime errors #104

Closed
obycode opened this issue Sep 25, 2023 · 7 comments
Closed

Handle runtime errors #104

obycode opened this issue Sep 25, 2023 · 7 comments
Assignees
Labels
missing Some missing functionality that is required for v1.0

Comments

@obycode
Copy link
Collaborator

obycode commented Sep 25, 2023

In initialize_contract and call_function, we'll need to handle runtime errors correctly. That includes mapping the traps from our standard library functions to the standard Clarity errors, and also mapping errors from host-interface functions back to clarity errors.

@github-project-automation github-project-automation bot moved this to Status: 🆕 New in Stacks Core Eng Sep 25, 2023
obycode added a commit to stacks-network/stacks-core that referenced this issue Sep 25, 2023
@obycode obycode moved this from Status: 🆕 New to Status: 📋 Backlog in Stacks Core Eng Oct 2, 2023
obycode added a commit to stacks-network/stacks-core that referenced this issue Nov 28, 2023
@obycode obycode added the missing Some missing functionality that is required for v1.0 label Jan 4, 2024
cylewitruk pushed a commit to stacks-network/stacks-core that referenced this issue Feb 8, 2024
@smcclellan
Copy link
Collaborator

@csgui to see if this is still an issue.

@smcclellan smcclellan added this to the WASM Phase 1 milestone Mar 12, 2024
@obycode
Copy link
Collaborator Author

obycode commented Mar 12, 2024

This is still an open item.

@csgui
Copy link
Collaborator

csgui commented Mar 13, 2024

Thanks @obycode

@csgui
Copy link
Collaborator

csgui commented Mar 15, 2024

Note: On the file below, some tests are being ignored due this issue. Remove ignored tag when it's fixed.

  • conditionals.rs

@obycode
Copy link
Collaborator Author

obycode commented Apr 8, 2024

In standard.wat, we have defined the function stdlib.runtime-error:

    ;; The error code is one of:
        ;; 0: overflow
        ;; 1: underflow
        ;; 2: divide by zero
        ;; 3: log of a number <= 0
        ;; 4: expected a non-negative number
        ;; 5: buffer to integer expects a buffer length <= 16
        ;; 6: panic
        ;; 7: short return
    (func $stdlib.runtime-error (param $error-code i32)
        ;; TODO: Implement runtime error
        unreachable
    )

This TODO is the goal of this issue. This function gets called whenever we need to trigger a runtime error during a wasm execution. The possible causes of these runtime errors are enumerated in the comment. If we trigger one of these runtime errors today, we will see the execution return something like:

Err(Wasm(Runtime(error while executing at wasm backtrace:
    0: 0x2e33 - <unknown>!stdlib.runtime-error
    1: 0x2ad8 - <unknown>!<wasm function 105>

Instead, we need to catch this wasm runtime error, and convert it to a clarity error, based on that error code that is passed into this function. For example:

Err(ShortReturn(ExpectedValue(Int(1))))

This should happen wherever we call a wasm function:

  1. In initialize_contract (clar2wasm/src/initialize.rs:369):
    top_level
      .call(&mut store, &[], results.as_mut_slice())
      .map_err(|e| Error::Wasm(WasmError::Runtime(e)))?;
  1. In call_function (clar2wasm/src/wasm_utils.rs:1286) (note the other TODO here from @krl):
    func.call(&mut store, &wasm_args, &mut results)
      .map_err(|e| {
          // TODO: If the root cause is a clarity error, we should be able to return that,
          //       but it is not cloneable, so we can't return it directly.
          //       If the root cause is a trap from our Wasm code, then we need to translate
          //       it into a Clarity error.
          //       See issue stacks-network/clarity-wasm#104
          // if let Some(vm_error) = e.root_cause().downcast_ref::<crate::vm::errors::Error>() {
          //     vm_error.clone()
          // } else {
          //     Error::Wasm(WasmError::Runtime(e))
          // }
          Error::Wasm(WasmError::Runtime(e))
      })?;

@obycode
Copy link
Collaborator Author

obycode commented Apr 8, 2024

After these errors are properly mapped, then we should remove the .map_err(|_| ()) in interpret and evaluate so that the errors between the two runtimes can be properly compared in the tests.

@smcclellan smcclellan moved this from Status: 📋 Backlog to Status: 💻 In Progress in Stacks Core Eng Apr 9, 2024
@csgui csgui moved this from Status: 💻 In Progress to Status: In Review in Stacks Core Eng Jun 10, 2024
@csgui csgui moved this from Status: In Review to Status: ✅ Done in Stacks Core Eng Jun 27, 2024
@csgui
Copy link
Collaborator

csgui commented Jun 27, 2024

Fixed by #379

@csgui csgui closed this as completed Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing Some missing functionality that is required for v1.0
Projects
Archived in project
Development

No branches or pull requests

3 participants