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

RFE: Add error-handling example for C API’s wasm_instance_new() #2472

Closed
FGasper opened this issue Jul 15, 2021 · 11 comments
Closed

RFE: Add error-handling example for C API’s wasm_instance_new() #2472

FGasper opened this issue Jul 15, 2021 · 11 comments
Labels
📚 documentation Do you like to read? 🎉 enhancement New feature! 📦 lib-c-api About wasmer-c-api

Comments

@FGasper
Copy link
Contributor

FGasper commented Jul 15, 2021

Motivation

When wasm_instance_new() fails, how should I consume traps if it’s non-NULL? Nothing seems to indicate how I can discover how many traps there are.

Proposed solution

Add examples that show how to report all of the available information about the failure when wasm_instance_new() fails.

@FGasper FGasper added the 🎉 enhancement New feature! label Jul 15, 2021
@FGasper
Copy link
Contributor Author

FGasper commented Jul 15, 2021

UPDATE: Per the Rust code it looks like the variable can only ever expose a single trap … so should it be renamed?

@Hywan Hywan added 📚 documentation Do you like to read? 📦 lib-c-api About wasmer-c-api labels Jul 17, 2021
@Hywan
Copy link
Contributor

Hywan commented Jul 17, 2021

Hello,

The naming follows the Wasm standard C API. However, you might be interested by #1744 and #1761. I agree that the documentation should contain more information about it.

Do you want to open a PR?

@FGasper
Copy link
Contributor Author

FGasper commented Jul 17, 2021

Hello,

The naming follows the Wasm standard C API. However, you might be interested by #1744 and #1761. I agree that the documentation should contain more information about it.

Do you want to open a PR?

I’ll comment on the naming in the C API repo, as that seems to be the source of the “naming bug”.

I can look at creating a PR to provide error handling in the C API examples, sure.

Thank you!

@FGasper
Copy link
Contributor Author

FGasper commented Jul 17, 2021

The naming follows the Wasm standard C API.

I don’t see anything in https://github.com/WebAssembly/wasm-c-api/blob/master/include/wasm.h that uses the word “traps” … ?

@syrusakbary
Copy link
Member

https://github.com/WebAssembly/wasm-c-api/blob/master/include/wasm.h#L376-L387

I may be confused... but there can be only one trap at a time.

UPDATE: Per the Rust code it looks like the variable can only ever expose a single trap … so should it be renamed?

This might be a good idea. What do you think will be good to rename?

@FGasper
Copy link
Contributor Author

FGasper commented Jul 17, 2021

What do you think will be good to rename?

I’m working on a PR just to rename them “trap”.

FGasper added a commit to FGasper/wasmer that referenced this issue Jul 17, 2021
bors bot added a commit that referenced this issue Jul 17, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
FGasper added a commit to FGasper/wasmer that referenced this issue Jul 17, 2021
bors bot added a commit that referenced this issue Jul 17, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
@FGasper
Copy link
Contributor Author

FGasper commented Jul 17, 2021

@syrusakbary @Hywan One of the challenges I’ve found is determining how best to convert a trap’s contents into a string. Extracting the message is straightforward enough, but it would be great to be able to extract names from the stack trace as well.

Do you have any insight into how to do this?

FGasper added a commit to FGasper/wasmer that referenced this issue Jul 17, 2021
bors bot added a commit that referenced this issue Jul 17, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
FGasper added a commit to FGasper/wasmer that referenced this issue Jul 18, 2021
bors bot added a commit that referenced this issue Jul 18, 2021
2478: Rename wasm_instance_new()’s “traps” argument to “trap”. r=syrusakbary a=FGasper

Issue #2472

# Description

This clarifies a bit how to properly use the error-handling mechanism in `wasm_instance_new()`, which only produces 0 or 1 traps, not 2+ as the name “traps” implies.

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Felipe Gasper <felipe@felipegasper.com>
@Hywan
Copy link
Contributor

Hywan commented Jul 19, 2021

See #2444 which has fixed a bug, and more importantly has added more documentation.

@Hywan Hywan closed this as completed Jul 19, 2021
@Hywan
Copy link
Contributor

Hywan commented Jul 19, 2021

@FGasper
Copy link
Contributor Author

FGasper commented Jul 19, 2021

Everything is documented here

// Do something with the trap.

^^ @Hywan Are there any examples of this? I don’t see any.

It’s straightforward enough to get the error message itself, but what is to be done with the origin and trace?

@Hywan
Copy link
Contributor

Hywan commented Aug 12, 2021

This part is not documented yet. wasm_trap_origin returns a wasm_frame_t, and wasm_trap_trace returns a fills out a wasm_frame_vec_t. Please open an issue for that specifically :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation Do you like to read? 🎉 enhancement New feature! 📦 lib-c-api About wasmer-c-api
Projects
None yet
Development

No branches or pull requests

3 participants