Stream: jco

Topic: Throwing exceptions from JS imports

James Mart (Mar 19 2024 at 15:31):

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.

James Mart (Mar 19 2024 at 15:48):

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).

Guy Bedford (Mar 19 2024 at 15:59):

good point, I've posted https://github.com/bytecodealliance/jco/issues/405 to track

James Mart (Mar 19 2024 at 19:45):

@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.

Guy Bedford (Mar 19 2024 at 19:47):

@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.

James Mart (Mar 19 2024 at 19:57):

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.

Guy Bedford (Mar 19 2024 at 20:00):

being able to throw errors to JavaScript callers is beneficial, and handled by this top-level error interpretation scheme

Guy Bedford (Mar 19 2024 at 20:00):

since the goal for component model embeddings is to provide idiomatic usage in different language environments

Guy Bedford (Mar 19 2024 at 20:01):

that is - traps are errors which are non-conformant with any possible top-level result error type

Guy Bedford (Mar 19 2024 at 20:01):

this just describes the desired behaviour though - right now we are a bit too flexible in these cases per the previous issue you posted

James Mart (Mar 19 2024 at 20:03):

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
}

James Mart (Mar 19 2024 at 20:09):

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.

Guy Bedford (Mar 19 2024 at 20:21):

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: Oct 23 2024 at 20:03 UTC