In jco transpiled components, when an imported function returns a result<> type, the generated trampoline function is wrapped in try/catch. This means if my import is filled with some js that throws an exception, it is caught:
try {
ret = { tag: 'ok', val: importedFunc(result0)};
} catch (e) {
ret = { tag: 'err', val: getErrorPayload(e) }; // <--- Thrown exception caught. `Val` has useful information.
}
But if the shape of the thrown object is not of the expected Err variant type, then a new generic TypeError is rethrown after the deserialization fails, which makes sense, but it masks the information about what was actually thrown, and my catch handler outside the call to the wasm doesn't get the originally thrown payload.
I think that the originally thrown object should be attached to the thrown TypeError object. This would allow external parsers to deal with the object that was actually thrown by the imported function.
Some other return types do not have the generated exception handling code, and therefore anything thrown from the import is directly propagated to the caller. I'm not sure which other types besides Result<ok, err> result in automatic exception handling code being generated.
I suppose the generic form of this request is:
All exceptions thrown from an import function should be propagated to the caller, in some way, even if it looks different depending on the expected return type of the import.
To my eye, the only exception to this should be when the thrown object exactly fits the shape of the expected Err variant of a result, in which case it can be internally handled by the wasm (which is already working).
good point, I've posted https://github.com/bytecodealliance/jco/issues/405 to track
@Guy Bedford It actually strikes me as slightly odd that returning the error variant from within a component causes JCO to throw a ComponentError. Shouldn't throw be for trapping? Returning an Err variant of the Result enum is a valid return value.
@James Mart errors are part of the component model. So my thinking is that traps should be used whenever the incorrect types are used - for both errors and normal values. I believe that gets us to the right kind of consistent behaviour here? That is, the bug is that errors of the wrong type should become traps. It's only conformant errors that get coerced into component model result errors. Throwing non-errors in JS is generally an anti-pattern so that this gap is open to us to use here.
I agree traps should be used when the incorrect types are used. But I'm no longer talking about the TypeError
, I'm suggesting there's a possible second issue here.
When I run a component, it returns an Err variant (conformant). But JCO throws to represent to the caller that the Err variant was returned.
if (variant5.tag === 'err') {
throw new ComponentError(variant5.val);
}
I don't feel strongly about this, but it does strike me as somewhat odd to throw this, rather than simply returning an object with the shape of the Err variant, given that the wasm never trapped. It is simply a type of valid response. My discomfort may just be due to my lack of familiarity with idiomatic js.
being able to throw errors to JavaScript callers is beneficial, and handled by this top-level error interpretation scheme
since the goal for component model embeddings is to provide idiomatic usage in different language environments
that is - traps are errors which are non-conformant with any possible top-level result error type
this just describes the desired behaviour though - right now we are a bit too flexible in these cases per the previous issue you posted
In my case, it now look like (in pseudocode):
try {
let returnValue = execute(component);
// process ok value
}
catch (e) {
if (e == trap) {
// trap! unrecoverable error
}
if (e == normal error) {
// process error value
}
}
Rather than
try {
let returnValue = execute(component);
if Ok(returnValue) {
// process ok value
} else if Err(returnValue) {
// process error value
}
}
catch (e) {
// Trap! unrecoverable error
}
My initial preference was for handling that looked more like the second code block above, since an Err variant is a valid response in my case. But I've understood from your response that the first block is correct & idiomatic.
Yes, that is the current design for idiomatic JS bindgen. You could do the explicit normal error checks before the trap checks since they will have the known shapes?
Last updated: Dec 23 2024 at 12:05 UTC