Implement extern "C" async functions.

[rodrigorc]

It converts a JS Promise into a wasm_bindgen_futures::JsFuture that
implements Future<Result<JsValue, JsValue>>.

Fixes issue #2189. Now you can write:

#[wasm_bindgen]
extern "C" {
    async fn js_test() -> Result<JsValue, JsValue>;
}

And the generated code will do the conversion automatically.

Currently there is the limitation that the return value must be Result<JsValue, JsValue>, because that is the Output type of a wasm_bindgen_futures::JsFuture.

I've added a custom error message if the user does not specify a return type, and let the compiler complaint if they write a different type.

I've tried doing automatic conversions to be able to return Result<i32, JsValue> for example, but I could make it work for every case. I think that these kind of conversions (unboxing?) is never done in the generated code. Maybe if wasm_bindgen_futures::JsFuture were generic on the return type...

I noticed that one of the test cases used a manually-written promise, so I abused a bit and converted it to the new format. I don't know if it is appropriate or if more tests are needed.

About the documentation, writing English is not my best talent, so feel free to rewrite it at will.

[alexcrichton]

Thanks for this! Before diving too much into the code though I think it'd be good to try to sort out the story for a generic return type here. For example it'd be great if async fn could work in the same way as defining an async function where you could omit the result, use any js-convertible types, etc. Would it be possible to mirror some of that conversion logic to automatically handle the Result<JsValue, JsValue> into a target T here? It might be necessary to define a new internal trait for that but I think that's ok.

[rodrigorc]

The automatic conversion of the return type of a promise has two separate issues.

The first one is that JS promises are always fallible, there are always the then and the catch callbacks. It is as if every extern async function has an implicit #[wasm_bindgen(catch)].
If an extern function is defined as async fn foo() -> T and the promise fails, the only sensible outcome would be to panic! (i.e. auto-unwrap). I don't know if that is a good thing to do automatically.

Currently a catch function requires a Result<...> return type, I would do the same here.

The second issue is the type of the Ok value itself. The tricky thing is that this is not an ABI type, because a JsFuture always returns a Result<JsValue, JsValue>. So if we write async fn foo() -> Result<i32, JsValue> then we have to convert the success JsValue into a i32, but what if it fails? I can think of three options:
a. panic!.
b. return i32::default().
c. use Result::and_then and do as if the promise failed.

I like "c". We could implement TryFrom<JsValue> for X for many common types. Those can be useful for the end user, too. There is the detail of if we want to do semantic casting or not (is_truthy vs as_bool, ...).

An extra issue is the conversion of the error type. I think that a #[wasm_bindgen(catch)] does a simple ?, so there is an implicit .into(). Here I think that we should do the same.

[rodrigorc]

Ok, I'm correcting myself. Any JS function may fail, just by doing throw "error", promises just have a fancier syntax for the catch. I think we can combine the wasm_bindgen(catch) attribute with the async to get the best of both worlds.

About how to handle the conversion errors, I just tried a few random normal functions returning the wrong value and sometimes they return default (0), sometimes they panic. I think we can do more or less the same.

I'll post some code not to be merged, just to get feedback (it misses docs and proper test cases).

BTW, do you prefer incremental commits or rebases in PRs?

[alexcrichton]

Looking good! I left some comments inline.

[alexcrichton]

Could this perhaps use TryFrom instead of a new trait? (now that I think about it).

It might be cool if this could leverage existing TryFrom implementation (and add a few here too), and that way if the conversion from JsValue fails we've got a way to return the error (it's like as if the future failed)

[rodrigorc]

Actually I tried that but it didn't go well. I wanted these blanket impls:

impl<T: JsCast> TryFrom<JsValue> for T { ... }
impl<T: TryFrom<JsValue>> TryFrom<JsValue> for Option<T> { ... }

But these resulted in a lot of conflicting implementations.

Maybe this trait could be leveraged into JsCast, but a new trait looked cleaner to me.

[alexcrichton]

Oh I don't think we'll be able to do some blanket impls, but we could add specific impls perhaps?

In general I'm just hoping that we can propagate an error for a failed conversion.

In general though this seems like a really tricky problem. If you'd prefer we can just stick to Result<JsValue, JsValue> like Promise and call it a day.

[rodrigorc]

Actually, returning Result<JsValue, JsValue> was my first option, but it looked poor. The user can just map to do the casting, but it breaks ergonomics.

And at first I though that I could coerce the error in the conversion to that Err(_) but it did not work either. On one side, because a cach function is not required to return a JsValue but any type that implements From<JsValue>, because of the ? operator. But a cast failure would be a js_sys::TypeError, and that conversion may not be possible.
On the other side, wasm_bindgen does not actually depend on js_sys. Adding that dependency just for this seemed overkill.

The fancy solution could be making JsFuture generic and doing ABI magic in the JS side, just like regular functions do. But that is way out of my league.

Anyway, I'm not convinced with the failing conversions argument. If I write a regular JS function and import it with a wrong prototype it will just panic or return a default value, depending on the types, (I think these situations are not documented):

function foo() { return 42; }

#[wasm_bindgen]
extern "C" {
    fn foo() -> String;  //undefined?
}

Going async would be quite similar:

async function foo() { return 42; }

#[wasm_bindgen]
extern "C" {
    async fn foo() -> String; //undefined!
}

[rodrigorc]

Hi again! I have rewritten this PR and rebased to a single commit (I hope you don't mind).
I think this is the best compromise between easy/future proof/sensible. The rules are:

• an export async function can return either a JsValue or nothing.
• async can be combined with catch, thus returning a Result<JsValue,_> or Result<(), _>.

If the user wants a different type, they have to do the casting themselves and dealing with the errors.
The error in the Result follows the same rules as that in any catch function.

This way you can write 4 kinds of extern async functions, based on the return type, that I think they are quite useful.

#[wasm_bindgen]
extern "C" {
    async fn func1();
    async fn func2() -> JsValue;
    #[wasm_bindgen(catch)]
    async fn func3() -> Result<JsValue, JsValue>;
    #[wasm_bindgen(catch)]
    async fn func4() -> Result<(), JsValue>;
}

There are still the test cases missing, and the doc changes need a reword...
What do you think?

[alexcrichton]

That all sounds great to me! If you want to add some tests and update the docs I think this should be good to go!

[rodrigorc]

Hi! I rebased yet again, this time with more or less proper docs and test cases.

While writing the test cases I noticed a bug when the functions is declared to return nothing:

#[wasm_bindgen(module = "tests/wasm/futures.js")]
extern "C" {
    #[wasm_bindgen]
    async fn call_promise_unit();
}

because the JS-glue generator, seeing that it returns nothing, skipped the function. But I need the JS-glue to be able to return the Future, so I added a line in DescribeImport::to_tokens() to handle that special case.

[alexcrichton]

All looks good to me, thanks again for this! I think rustfmt needs to run again but otherwise should be good to go?

[rodrigorc]

Oh, I ran rustfmt to the test cases, but it kept on removing the async from the extern "C" block, and then I grew tired of it.

I'm quite happy with this version, but sorry, I don't know the procedure... do you want me to do the rustfmt or will you do you do it?

[alexcrichton]

Oh that may be a bug in rustfmt then, mind filing an issue upstream and adding #[rustfmt::skip] there for now?

(CI is failing and we gotta get it green somehow)

[rodrigorc]

Green light (I hope)! Issue created at rustfmt.

[alexcrichton]

👍

Thanks!