Skip to content

Unhandeled error in SC runtime #357

Description

@peterjah

original issue: massalabs/massa#4923

Digging into this: the assumption that it is the nested-call limit is wrong, and so is the "you have more than 25 cascaded calls" answer. The Depth error: prefix here is a mislabel, not a diagnosis.

What the message actually decomposes into

The observed string is the concatenation of four Display impls:

Fragment Variant Crate
VM Error in CallSC context: {error} ExecutionError::VMError massa-execution-exports/src/error.rs:57
Depth error: {0} VMError::DepthError massa-sc-runtime src/error.rs:12
VM error: {0} ABIError::VMError massa-sc-runtime src/as_execution/error.rs:16
VM instance error: {0} VMError::InstanceError massa-sc-runtime src/error.rs:8
RuntimeError: unreachable\n at <unnamed> … wasmer::RuntimeError wasmer

So the innermost cause is a plain wasm unreachable trap in the guest (AssemblyScript abort / unhandled failure) inside a nested call, which then got relabelled Depth error on the way out.

Why it gets relabelled

massa-sc-runtime/src/error.rs:

impl From<wasmer::RuntimeError> for VMError {
    fn from(e: wasmer::RuntimeError) -> Self {
        if let Some(err) = e.downcast_ref::<ABIError>() {
            VMError::DepthError(err.to_string())   // <-- unconditional
        } else {
            VMError::InstanceError(e.to_string())
        }
    }
}

Any ABIError that trapped out of a host function is turned into DepthError, regardless of which variant it is. The propagation chain for the reported case:

  1. A nested call runs a module that traps with unreachable.
  2. From<wasmer::RuntimeError> for VMError — downcast fails (it's a real wasm trap, not an ABIError) → VMError::InstanceError("RuntimeError: unreachable …").
  3. call_module (src/as_execution/common.rs:41-56) propagates it with ? → From<VMError> for ABIError → ABIError::VMError(…).
  4. That ABIError is returned from the ABI host function, so wasmer wraps it in a RuntimeError carrying the ABIError payload.
  5. At the outer frame the downcast now succeeds, and the branch above stamps DepthError on it.

A genuine depth error looks different — increment_recursion_counter builds InterfaceError::DepthError("recursion depth limit reached"), and ABIError::DepthError has Display = "{0}", so it comes out clean:

VM Error in CallSC context: Depth error: recursion depth limit reached

which is exactly what massa-execution-worker/src/tests/scenarios_mandatories.rs:794 asserts. The reported message has an extra VM error: VM instance error: sandwich, which a real depth error can never produce.

Consequence

Every error raised from inside a nested call is reported to the user as a "Depth error". That sends developers hunting for a recursion-limit problem when the actual fault is in their contract (an abort, an out-of-bounds access, a failed assertion) one frame down. The original cause is preserved in the string but buried under a wrong label.

Suggested fix (in massa-sc-runtime)

Preserve the variant instead of flattening it:

impl From<wasmer::RuntimeError> for VMError {
    fn from(e: wasmer::RuntimeError) -> Self {
        match e.downcast_ref::<ABIError>() {
            Some(ABIError::DepthError(msg)) => VMError::DepthError(msg.clone()),
            Some(err) => VMError::InstanceError(err.to_string()),
            None => VMError::InstanceError(e.to_string()),
        }
    }
}

This keeps Depth error: recursion depth limit reached intact for the real case (existing test at scenarios_mandatories.rs:794 still passes) and reports everything else under its own label. On top of that, the depth message could name the limit, e.g. recursion depth limit reached (max 25 nested calls), since max_recursive_calls_depth is available at that point.

Note this is a message/labelling change only — no consensus-observable behavior changes (the same executions still fail, with the same gas), so no MIP is needed. It does change the string in ExecutionOutput events/errors, so anything matching on "Depth error" in tooling would need updating.

Still open as a separate question: whether the wasm backtrace (at <unnamed> (<module>[14]:0x6f3)) is useful to surface at all, given it carries no symbol names.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions