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

Expose wasmtime_runtime::InstantiationError as a stable wasmtime API #3928

Open
pchickey opened this issue Mar 14, 2022 · 2 comments
Open

Expose wasmtime_runtime::InstantiationError as a stable wasmtime API #3928

pchickey opened this issue Mar 14, 2022 · 2 comments
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime Issues about wasmtime that don't fall into another label

Comments

@pchickey
Copy link
Contributor

Feature

Presently, the instantiate family of functions (Linker::instantiate, InstancePre::instantiate, their _async cousins etc) error with an anyhow::Error. To inspect those errors, we can try downcasting to wasmtime_runtime::InstantiationError.

Benefit

Users who need to inspect those errors need to keep their deps of wasmtime and wasmtime_runtime in sync. wasmtime_runtime's API is not designed for stable use. Stabilizing this error API gets rid of a possible runtime dep for users.

The InstantiationError::Limit variant is used when a wasmtime pooling allocator is out of instances. This is the only way that wasmtime crate users can observe this condition. There should be a stable way to observe this.

The InstantiationError::Trap variant contains a wasmtime_runtime::Trap, which is different from a wasmtime::Trap in ways that aren't useful to wasmtime users, and can be confusing.

Implementation

I think it makes sense for the instantiate family of functions to still error with an anyhow::Error, but wasmtime should export types to try downcasting that error to.

Wasmtime should not re-export wasmtime_runtime::InstantiationError because it exposes the wrong sort of Trap. Instead it should map the variants to types which are in the public API.

pub enum InstantiationError {
    Resource(anyhow::Error),
    Link(LinkError),
    Trap(Trap),
    Limit(u32),
}
  • Resource variant: just return the anyhow::Error here.
  • Link variant: re-export wasmtime_runtime::LinkError for downcasting to that variant, since this is just a string wrapper.
  • Trap variant: map the contents to a wasmtime::Trap.
  • Limit variant: Wasmtime could define a new public type PoolingAllocatorLimit and impl Error on it, and map the InstantiationError::Limit variant to that type.

Alternatives

There are probably other good ideas I haven't thought of here! I am very open to suggestions.

@alexcrichton
Copy link
Member

Personally I'm hesitant to expose error-enums because in the limit every function would needs its own error enum. For example InstantiatePre::instantiate can't return an InstantiationError::Link, but Instance::new and Linker::instantiate can. There's also a number of other cases like Func::call which can fail for any number of reasons but we lump them all into anyhow::Error.

That being said though it definitely makes sense to want to work with specific kinds of errors and handle those differently than others. For that though we've relied on downcasting where possible. For example for instantiations that fail because of a trap you should be able to downcast_ref::<wasmtime::Trap>() and get the trap out. I don't think we have downcasts for other errors though.

In that sense I would personally prefer to have specific errors you can downcast to instead of having specific errors returned from each API. Is there a particular error you're thinking of looking for beyond traps?

@pchickey
Copy link
Contributor Author

I think we are in agreement, and I just wasn't clear in the language I used for the proposal above. The error I need most is the Limit error, but I figured we could cover all four variants pretty easily.

@alexcrichton alexcrichton added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime Issues about wasmtime that don't fall into another label labels Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime Issues about wasmtime that don't fall into another label
Projects
None yet
Development

No branches or pull requests

2 participants