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

feat(wasmtime): add error hint for missing WASI imports #10259

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vados-cosmonic
Copy link
Contributor

This commit adds some code to print an error hint when a failed wasmtime run command could have been called with arguments (in particular -S <capability>) that would have enabled the command to succeed.

Output looks like this, when running a module that imports wasi:http:

warning: missing import [wasi:http/[email protected]] has a WASI import option [http]  that has not been enabled -- consider re-running with '-S http'.
Run `wasmtime -S help` for a full list of WASI import options.

Error: failed to run main module `http_hello_world.wasm`

Caused by:
    0: component imports instance `wasi:http/[email protected]`, but a matching implementation was not found in the linker
    1: instance export `fields` has the wrong type

There are a few bits left to figure out (ex. seems like wit-parser doesn't yet have any shared/reusable relatively simple function for parsing import names into constituent parts?), but wanted to put up a draft to get some feedback, and make sure this is even desirable for maintainers.

This commit adds some code to print an error hint when a failed
`wasmtime run` command could have been called with arguments (in
particular `-S <capability>`) that would have enabled the command to succeed.

Signed-off-by: Victor Adossi <[email protected]>
@vados-cosmonic vados-cosmonic force-pushed the feat(wasmtime)=add-error-hint-for-missing-wasi-flags branch from fadc897 to 51d8d52 Compare February 20, 2025 18:55
@alexcrichton
Copy link
Member

Personally I'm always a fan of improving error messages, so overall seems like a good idea.

For the specifics though I'd prefer to try to get more structured data here as opposed to having stringly-typed error messages with string searching for when to print an error. For example we could add a dedicated error type which indicates exactly which import was not found (e.g. ComponentImportNotFound or something like that) to extract the missing one. Matching that to a WASI interface, and then to a flag, is going to be a bit trickier and that's where some more "stringly-typed" things may need to come into play (or just rough heuristics).

Using wit-parser here I'll note that the dependency doesn't already exist at runtime. There's additionally not WIT embedded in the CLI itself. In theory the generate! macro could generate extra structures to detect "is this import supplied by this" but then that also runs into version numbers and matching and such.

Basically I think it'll be unfortunately pretty nontrivial to present a good error message here, so to improve things it's probably necessary to accept false positives/negatives for the time being.

@vados-cosmonic
Copy link
Contributor Author

For the specifics though I'd prefer to try to get more structured data here as opposed to having stringly-typed error messages with string searching for when to print an error. For example we could add a dedicated error type which indicates exactly which import was not found (e.g. ComponentImportNotFound or something like that) to extract the missing one. Matching that to a WASI interface, and then to a flag, is going to be a bit trickier and that's where some more "stringly-typed" things may need to come into play (or just rough heuristics).

Yeah I definitely agree that it'd be better to have a structured error here -- given that we don't yet have a unified way to parse/represent the text of an import name (AFAIK, outside a Resolve), I figured that might be a bit difficult to get perfectly (we might end up passing a string anyway as the import name?).

Basically I think it'll be unfortunately pretty nontrivial to present a good error message here, so to improve things it's probably necessary to accept false positives/negatives for the time being.

Yeah what do you think would be a good way to proceed here? To just print the message regardless of error contents, maybe as a general hint ?

@alexcrichton
Copy link
Member

A possible idea:

  • Add a dedicated type to wasmtime::component::Linker where the error type represents "this failed to link because some import was missing" or something like that.
  • The CLI detects this case and then falls back to something to possibly add more context to the error that wasmtime::component::Linker generated
  • This "add context" method would look something along the lines of:
    • Enable all -S... options and attempt to create an InstancePre. If that fails then don't add context and just return an error.
    • If that succeeds then for every -S option turn it back off and see if creation of InstancePre still fails. If creation succeeds, leave it off, otherwise leave it on.
    • At the end diff -S options with what the user provided, adding context to the error saying "did you mean -Scli,http" or similar

That's a little involved but in theory should be pretty robust in the face adding/removing methods over time and such. It'd also handle the case of something like -Sexit-with-code which is pretty different from cases of "the entire interface is missing"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants