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

Returning Result<Vec<u8>, JsError> from an async function doesn't work #3155

Closed
nasso opened this issue Nov 17, 2022 · 11 comments · Fixed by #3630
Closed

Returning Result<Vec<u8>, JsError> from an async function doesn't work #3155

nasso opened this issue Nov 17, 2022 · 11 comments · Fixed by #3630
Assignees
Labels

Comments

@nasso
Copy link
Contributor

nasso commented Nov 17, 2022

Describe the Bug

The following compiles:

#[wasm_bindgen]
pub fn foo() -> Result<Vec<u8>, js_sys::Error> {
  unimplemented!()
}

While the following does not:

#[wasm_bindgen]
pub async fn foo() -> Result<Vec<u8>, js_sys::Error> {
  unimplemented!()
}

Steps to Reproduce

  1. Copy and paste the second snippet above
  2. cargo check
  3. See error

Expected Behavior

Whatever works without async works the same with async.

Actual Behavior

Sometimes, it does not 😔.

Additional Context

Not the first instance of this issue (see #3059).

@nasso nasso added the bug label Nov 17, 2022
@Teebor-Choka
Copy link

This issue is unpleasant. Any chance of extending to functionality for async to match the sync behavior?

@daxpedda
Copy link
Collaborator

I didn't look into it, but I don't think this requires any design work, so PRs are welcome.

@Teebor-Choka
Copy link

I created a custom type to wrap around the Vec. Was asking about future prospects.

@daxpedda
Copy link
Collaborator

daxpedda commented Jun 1, 2023

I'm not aware of any current maintainers interested in working on something like this.

@benma
Copy link

benma commented Jul 5, 2023

Fyi I ran into the same and would welcome a fix. For String it works both with async fn and regular fn, but not with Vec.

@Solo-steven
Copy link
Contributor

Hi @daxpedda I would like to work on this bug if no one is working on it! Thanks ~

@daxpedda
Copy link
Collaborator

Go right ahead 😃!

@Solo-steven
Copy link
Contributor

Solo-steven commented Sep 20, 2023

Hi @daxpedda this currently solution I can think about is like

impl<T> From<Vec<T>> for JsValue {
    #[inline]
    fn from(s: Vec<T>) -> JsValue {
        JsValue::from(s.as_ptr() as usize)
    }
}

but this would first convert the raw pointer into f64 ,

    /// Creates a new JS value which is a number.
    ///
    /// This function creates a JS value representing a number (a heap
    /// allocated number) and returns a handle to the JS version of it.
    #[inline]
    pub fn from_f64(n: f64) -> JsValue {
        unsafe { JsValue::_new(__wbindgen_number_new(n)) }
    }

then pass it to the JS function which adds it to heap

module.exports.__wbindgen_number_new = function(arg0) {
    const ret = arg0;
    return addHeapObject(ret);
};
function addHeapObject(obj) {
    if (heap_next === heap.length) heap.push(heap.length + 1);
    const idx = heap_next;
    heap_next = heap[idx];

    heap[idx] = obj;
    return idx;
}

I do not know if the behavior is correct or not, since it just adds the address of vec to the JS heap, but in wasm-bindgen guide, it seems to be ok to convert type into a number, because it just abi ? Any help would be very appreciate !


It seems like we need to convert vec into JS Array ?
reference

@Liamolucko
Copy link
Collaborator

Hi @daxpedda I would like to work on this bug if no one is working on it! Thanks ~

Oh, I'm actually already partway through implementing this. I'd forgotten that this issue existed, I was working on it because I was fixing the issue @cschramm pointed out in #3554 (comment) that #3554 didn't cover async functions. But I should probably put more effort into making sure I've self-assigned myself to anything I'm working on.

@daxpedda
Copy link
Collaborator

Sorry @Solo-steven, poor communication on our part here ...

@Solo-steven
Copy link
Contributor

It is OK, and thanks @Liamolucko for giving me the link to related PR so that I can learn from it! thanks

Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Sep 21, 2023
This means that `Vec`s can now be returned from `async` functions.

Fixes rustwasm#3155.

I had to add a new `VectorIntoJsValue` trait, since similarly to
`VectorIntoWasmAbi` the orphan rule won't let us directly implement
`From<Vec<T>>` for `JsValue` outside of `wasm-bindgen`.
Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Mar 24, 2024
This means that `Vec`s can now be returned from `async` functions.

Fixes rustwasm#3155.

I had to add a new `VectorIntoJsValue` trait, since similarly to
`VectorIntoWasmAbi` the orphan rule won't let us directly implement
`From<Vec<T>>` for `JsValue` outside of `wasm-bindgen`.
Liamolucko added a commit that referenced this issue Mar 25, 2024
* Implement `Into<JsValue>` for `Vec`

This means that `Vec`s can now be returned from `async` functions.

Fixes #3155.

I had to add a new `VectorIntoJsValue` trait, since similarly to
`VectorIntoWasmAbi` the orphan rule won't let us directly implement
`From<Vec<T>>` for `JsValue` outside of `wasm-bindgen`.

* Implement Into<JsValue> for boxed slices of numbers as well

* Add changelog entry

* Fix memory leak

* Add missing if_std!

* Move the changelog entry to the right spot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants