fix: host_bindgen! returns Result instead of panicking on guest errors#1317
fix: host_bindgen! returns Result instead of panicking on guest errors#1317jsturtevant wants to merge 2 commits intomainfrom
Conversation
The host_bindgen! macro generated export wrapper functions that would
panic when Callable::call() returned Err (e.g., guest abort, trap,
timeout). This crashed the host process.
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
21ca91e to
eadb9f1
Compare
ludfjig
left a comment
There was a problem hiding this comment.
I think I am fine with this, but I suspect this is intentional. I think we generally want to avoid Result types as much as possible where it makes sense, but since this matches the current guest calling convention maybe let's defer that discussion to later
Open to other ways of addressing this I guess but a panic in the guest shouldn't panic this host. It seemed practical to match existing behavoir |
|
I agree with @ludfjig that, generally speaking, the semantic gap of lifting everything from the interface into Option isn't particularly attractive. However, I do see the need to deal with errors that fundamentally can happen during execution on the host side. Is there a reason that this is also applied to the host functions imported by the guest? I think that generally speaking it is the ability to export a function implementing an interface, but actually implement a different interface (i.e. one with more optionality) that is the most semantically damaging---so the implicit Let me know what you think about the usability of that. As I think about this more, from the wasm perspective especially, I do think that having the |
fixes: #1316
The host_bindgen! macro generated export wrapper functions that called panic! when Callable::call() returned Err (e.g., guest abort, trap, timeout). This crashed the host process instead of letting callers handle the error gracefully.
Breaking Change
Host-side trait implementations generated by host_bindgen! must now return Result<T, HyperlightError> instead of bare T. Existing implementations need to wrap return values in Ok(...). Constructors (fn new) are unaffected.
This more closely matches the non-wit function calls which have errors that are returned.
Signed-off-by: James Sturtevant jsturtevant@gmail.com