Stream: git-wasmtime

Topic: wasmtime / PR #7645 Better error message for wrong import...

Wasmtime GitHub notifications bot (Dec 06 2023 at 08:37):

rylev requested pchickey for a review on PR #7645.

Wasmtime GitHub notifications bot (Dec 06 2023 at 08:37):

rylev opened PR #7645 from rylev:better-error to bytecodealliance:main:

This changes improve the error message when a component is importing/exporting something that the linker is either missing or has the wrong type.

Previously the error message for a missing function implementation would be:

Error: import `my-import` has the wrong type

Caused by:
    expected func found nothing

Now, this error will be:

Error: component imports `my-import`, but a correct implementation of `my-import` could not be found

Caused by:
    function implementation is missing

I went back and forth on whether the error message should reference the linker. For direct users of Linker this would surely help, but this error message also gets displayed to indirect users of Linker so talking at a higher level of abstraction seemed appropriate.

Wasmtime GitHub notifications bot (Dec 06 2023 at 08:37):

rylev requested wasmtime-core-reviewers for a review on PR #7645.

Wasmtime GitHub notifications bot (Dec 06 2023 at 10:06):

rylev updated PR #7645.

Wasmtime GitHub notifications bot (Dec 06 2023 at 12:50):

github-actions[bot] commented on PR #7645:

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 (Mar 07 2024 at 15:27):

peterhuene submitted PR review:

I think what you have is definitely trending in the right direction for an easier to understand error message.

Just a few feedback comments.

Additionally, "implementation" in some of the errors rubs me the wrong way for some reason I can't seem to articulate, nor can I think of a better word to use.

Wasmtime GitHub notifications bot (Mar 07 2024 at 15:27):

peterhuene submitted PR review:

I think what you have is definitely trending in the right direction for an easier to understand error message.

Just a few feedback comments.

Additionally, "implementation" in some of the errors rubs me the wrong way for some reason I can't seem to articulate, nor can I think of a better word to use.

Wasmtime GitHub notifications bot (Mar 07 2024 at 15:27):

peterhuene created PR review comment:

                format!("component imports {desc} `{name}`, but a matching implementation was not found", desc = ty.desc())

With an additional implementation of TypeDef::desc to say what it is, e.g. "instance", "type", "function", etc.

Wasmtime GitHub notifications bot (Mar 07 2024 at 15:27):

peterhuene created PR review comment:

Unfortunately TypeDef::Interface is misnamed, it's actually representing a type so the message should indicate a type.

If it's not too invasive, I think we should probably change Interface to Type too.

Wasmtime GitHub notifications bot (Mar 07 2024 at 15:27):

peterhuene created PR review comment:

                None => bail!("module implementation is missing"),

Wasmtime GitHub notifications bot (Mar 07 2024 at 16:26):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Mar 07 2024 at 16:26):

peterhuene created PR review comment:

Actually, probably not worth taking a breaking change to rename the enum variant (I think the only external consumer of it is jco, but there could be others); let's leave it Interface and just change the error message here back to indicate a "type".

Wasmtime GitHub notifications bot (Mar 11 2024 at 10:05):

rylev updated PR #7645.

Wasmtime GitHub notifications bot (Mar 11 2024 at 10:08):

rylev commented on PR #7645:

@peterhuene I believe I've addressed your feedback. I agree that "implementation" isn't always the right word, but I think we'll need to let this sit for a while before the right word makes itself apparent.

Wasmtime GitHub notifications bot (Mar 11 2024 at 10:09):

rylev requested peterhuene for a review on PR #7645.

Wasmtime GitHub notifications bot (Mar 11 2024 at 10:26):

rylev updated PR #7645.

Wasmtime GitHub notifications bot (Mar 11 2024 at 18:18):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Mar 11 2024 at 18:49):

peterhuene merged PR #7645.

Last updated: Nov 22 2024 at 17:03 UTC