wasmtime / issue #5098 Make it possible to get user's con... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #5098 Make it possible to get user's con...

Wasmtime GitHub notifications bot (Oct 22 2022 at 02:17):

github-actions[bot] commented on issue #5098:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Oct 24 2022 at 14:54):

alexcrichton commented on issue #5098:

Personally I'm a bit hesitant to dive into this world of errors and downcasting and trying to make Trap look like an anyhow::Error, mostly because we'll probably just have a long tail of feature requests. I wonder, though, if we could perhaps make a slight change to the embedding APIs and solve this? For example I think the underlying issue here is the ability for an embedder to return an error in a host function and get back the original error on "the other side". I think that could be possible with:

Change all host functions to return anyhow::Error as the Result error instead of Trap

Remove the From impls for Trap for error-looking things

Change all Wasmtime APIs that return Result<_, Trap> to instead return anyhow::Result<_>

That would effectively mean that we punt all error handling to anyhow and everything is plumbed through there rather than having to go through Trap. Traps would then always be a "leaf" error which wouldn't contain anything other than a message -- and we could perhaps even do away with that since there's probably no need for a host-created Trap at that point any more.

While it would be a more impactful change in terms of embedding APIs at least to me it feels more sustainable over time than slowly moving much of anyhow::Error into Trap which. Basically, in retrospect, I think that the current design of Trap may be a mistake.

Wasmtime GitHub notifications bot (Oct 24 2022 at 18:21):

pchickey commented on issue #5098:

I spent some time thinking about this and I totally agree we should try changing the design to use anyhow everywhere and see how it goes. For instance, it has the additional knock-on effect that the i32 exit can become a concrete user error struct I32Exit(i32) belonging to the wasi-common crate, something we all wanted since the i32 variant was added to traps when Wasi was created.

However, I'm worrying about prioritizing this redesign right now, because it will involve touching a lot of crates, tests, and downstream consumers of wasmtime. This project isn't on the critical path for our current thrust to make the component model a usable thing. So, with that in mind, I'd rather merge this non-breaking (except for the Box error thing) change now, and then circle back and revisit the whole design a bit later.

Wasmtime GitHub notifications bot (Oct 24 2022 at 20:37):

pchickey commented on issue #5098:

It occurred to me that we only need to merge this PR to get the benefits of moving WASI to a concrete exit type, so I guess thats an additional argument towards merging - it makes our migration path clearer

Wasmtime GitHub notifications bot (Oct 24 2022 at 20:37):

pchickey edited a comment on issue #5098:

It occurred to me that we only need to merge this PR to get the benefits of moving WASI to a concrete exit type, so I guess thats an additional argument towards merging - it makes our migration path incremental

Wasmtime GitHub notifications bot (Oct 24 2022 at 20:55):

alexcrichton commented on issue #5098:

I'm ok taking a more expedient route, but I wouldn't expect this to have much "complicated" churn with it since signatures can largely "just" be updated and ? should continue to work as usual. I realize though that the churn, while easy to handle, can be large and cumbersome.

As-is though this'll need to change a little bit since when printing a stack of errors this'll double-print the error that the trap represents with anyhow::Error. It'll be printed by Display for Trap and then it will additionally be printed by the .source() since that returns the same trap. I think that we would at least need to fix that before landing, even in an interim state.

Wasmtime GitHub notifications bot (Oct 26 2022 at 22:12):

pchickey commented on issue #5098:

It appears to me that the trap source only gets printed twice with println!("{:?}", anyhow::Error::from(the_trap)) - anyhow's display impl doesn't print the source. I don't think we should make Trap's Display impl less useful when the only benefit is to improve an anyhow::Error wrapper's Debug impl.

use anyhow::Result;
use wasmtime::*;

fn main() -> Result<()> {
    let mut store = Store::<()>::default();
    let wat = r#"
        (module
        (func $hello (import "" "hello"))
        (func (export "run") (call $hello))
        )
    "#;

    #[derive(Debug)]
    struct MyTrap;
    impl std::fmt::Display for MyTrap {
        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
            write!(f, "my trap")
        }
    }
    impl std::error::Error for MyTrap {}

    let module = Module::new(store.engine(), wat)?;
    let hello_type = FuncType::new(None, None);
    let hello_func = Func::new(&mut store, hello_type, |_, _, _| {
        Err(Trap::from(anyhow::Error::from(MyTrap)))
    });

    let instance = Instance::new(&mut store, &module, &[hello_func.into()])?;
    let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?;

    let e = run_func
        .call(&mut store, ())
        .err()
        .expect("error calling function");
    println!("display run_func's err: {}", e);
    println!("debug run_func's err: {:?}", e);

    let source = std::error::Error::source(&e).expect("trap has a source");
    println!("display err's source: {}", source);
    println!("debug err's source: {:?}", source);

    source
        .downcast_ref::<MyTrap>()
        .expect("source downcasts to MyTrap");

    drop(source);

    let a = anyhow::Error::from(e);
    println!("display run_func's anyhow'd err: {}", a);
    println!("debug run_func's anyhow'd err: {:?}", a);

    let source = a.source().expect("anyhow trap has a source");
    println!("display anyhow'd err's source: {}", source);
    println!("debug anyhow'd err's source: {:?}", source);

    Ok(())
}

display run_func's err: my trap
wasm backtrace:
    0:   0x2c - <unknown>!<wasm function 1>

debug run_func's err: Trap { reason: Error(my trap), wasm_trace: [FrameInfo { module_name: None, func_index: 1, func_name: None, func_start: FilePos(43), instr: Some(FilePos(44)), symbols: [] }], runtime_trace: Backtrace([Frame { pc: 140485629587494, fp: 140726870698496 }]) }
display err's source: my trap
debug err's source: MyTrap
display run_func's anyhow'd err: my trap
wasm backtrace:
    0:   0x2c - <unknown>!<wasm function 1>

debug run_func's anyhow'd err: my trap
wasm backtrace:
    0:   0x2c - <unknown>!<wasm function 1>


Caused by:
    my trap
display anyhow'd err's source: my trap
debug anyhow'd err's source: MyTrap

Wasmtime GitHub notifications bot (Oct 26 2022 at 22:14):

pchickey edited a comment on issue #5098:

use anyhow::Result;
use wasmtime::*;

fn main() -> Result<()> {
    let mut store = Store::<()>::default();
    let wat = r#"
        (module
        (func $hello (import "" "hello"))
        (func (export "run") (call $hello))
        )
    "#;

    #[derive(Debug)]
    struct MyTrap;
    impl std::fmt::Display for MyTrap {
        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
            write!(f, "my trap")
        }
    }
    impl std::error::Error for MyTrap {}

    let module = Module::new(store.engine(), wat)?;
    let hello_type = FuncType::new(None, None);
    let hello_func = Func::new(&mut store, hello_type, |_, _, _| {
        Err(Trap::from(anyhow::Error::from(MyTrap)))
    });

    let instance = Instance::new(&mut store, &module, &[hello_func.into()])?;
    let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?;

    let e = run_func
        .call(&mut store, ())
        .err()
        .expect("error calling function");
    println!("display run_func's err: {}", e);
    println!("debug run_func's err: {:?}", e);

    let source = std::error::Error::source(&e).expect("trap has a source");
    println!("display err's source: {}", source);
    println!("debug err's source: {:?}", source);

    source
        .downcast_ref::<MyTrap>()
        .expect("source downcasts to MyTrap");

    drop(source);

    let a = anyhow::Error::from(e);
    println!("display run_func's anyhow'd err: {}", a);
    println!("debug run_func's anyhow'd err: {:?}", a);

    let source = a.source().expect("anyhow trap has a source");
    println!("display anyhow'd err's source: {}", source);
    println!("debug anyhow'd err's source: {:?}", source);

    source
        .downcast_ref::<MyTrap>()
        .expect("source downcasts to MyTrap");

    Ok(())
}

display run_func's err: my trap
wasm backtrace:
    0:   0x2c - <unknown>!<wasm function 1>

debug run_func's err: Trap { reason: Error(my trap), wasm_trace: [FrameInfo { module_name: None, func_index: 1, func_name: None, func_start: FilePos(43), instr: Some(FilePos(44)), symbols: [] }], runtime_trace: Backtrace([Frame { pc: 140485629587494, fp: 140726870698496 }]) }
display err's source: my trap
debug err's source: MyTrap
display run_func's anyhow'd err: my trap
wasm backtrace:
    0:   0x2c - <unknown>!<wasm function 1>

debug run_func's anyhow'd err: my trap
wasm backtrace:
    0:   0x2c - <unknown>!<wasm function 1>


Caused by:
    my trap
display anyhow'd err's source: my trap
debug anyhow'd err's source: MyTrap

Wasmtime GitHub notifications bot (Oct 27 2022 at 15:13):

alexcrichton commented on issue #5098:

Right yeah the hit is to Debug for anyhow::Error, although that's the primary mechanism by which errors are displayed AFAIK for anyhow, notably used by the CLI for example.

If you'd prefer I'd be happy to take on the change to use anyhow::Error everywhere and remove the ability for Trap to contain arbitrary host errors, personally I think that's the best way forward without contorting ourselves too much internally within wasmtime::Trap

Wasmtime GitHub notifications bot (Oct 27 2022 at 16:30):

pchickey commented on issue #5098:

OK, that works for me. I took a whack at it the other day for about 90 minutes and got sidetracked into wiggle. I'd rather go mess with snapshot1.wasm today than traps :)

Last updated: Nov 22 2024 at 17:03 UTC