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

How about impl AsyncRead/AsyncWrite for WebSocket? #25

Closed
najamelan opened this issue Mar 15, 2019 · 8 comments · Fixed by #379
Closed

How about impl AsyncRead/AsyncWrite for WebSocket? #25

najamelan opened this issue Mar 15, 2019 · 8 comments · Fixed by #379

Comments

@najamelan
Copy link

Motivation

Would allow frontend and backend code to communicate in an abstract manner on an async Stream. It allows code reuse and prevents every application to have to implement this on top of the existing websocket api

Proposed Solution

Add impl AsyncRead/AsyncWrite to websocket

Alternatives

Allow upgrading an Http connection to an AsyncRead/AsyncWrite without websockets. I have no idea what the implementation of this would look like, and if it's even possible without websockets.

That being said, this alternative would be preferable I think. It means we don't need WS on the server either, just an upgraded http connection.

Additional Context

I kind of need this. If need be I don't mind attempting to implement this myself. If this is deemed a good solution, is there any mentoring possible to hint the way?

@alexcrichton alexcrichton transferred this issue from rustwasm/wasm-bindgen Mar 15, 2019
@alexcrichton
Copy link
Contributor

alexcrichton commented Mar 15, 2019

Thanks for the report! I've transferred this over to the gloo repository here from the wasm-bindgen repository becaues the web_sys crate is intended to be a low level binding to web APIs, but this would be perfect for the gloo project!

@najamelan
Copy link
Author

najamelan commented Mar 15, 2019

@alexcrichton ok, that's cool. Currently what I'm looking at is using the set_onmessage on a websocket. If I understand well, the way to do this is by using a wasm-bindgen::Closure to create the callback Function.

I would have to expose something that has connect( url ) and returns an async Stream... Does that sound like the right approach to you? Does that seem realistic or do you see some technical blockers for this?

I also wonder, if I understood well, data from javascript has to be copied into the wasm memory. Is there a way to avoid this extra copy on the data that comes in from the websocket?

@fitzgen
Copy link
Member

fitzgen commented Mar 15, 2019

See also #7

@najamelan
Copy link
Author

I'm making good progress on this I think, but I need a bit of advice. There isn't all that much example code or docs for this stuff. I can build streams that yield MessageEvent, but then the only way I see from the docs to get the data out of there and in rust is by using into_serde. This will use json stringify... I doubt that's what we want. There seem to be ways to share memory pointers from javascript like with WebAssembly.Memory and such, but what is the best way to get a bytearray out of a websocket connection into rust with wasm-bindgen, wasm-pack? Do I just write custom javascript for this? Given that this is a rust library, how do we ship the js?

@najamelan
Copy link
Author

Never mind, I found how to make a UInt8Array out of it and use copy_to. I suppose it is currently impossible to receive data directly in the wasm memory to avoid copying?

@alexcrichton
Copy link
Contributor

In the spirit of abstraction it might be pretty cool to get a stream of MessageEvent structures from a websocket which could be further layered with abstractions to parse MessageEvent as JSON and things like that perhaps? I don't have too much of an opinion here, just some random thoughts!

I'm not too familiar with the websocket interface, but if you can specify a buffer to fill in then you can fill in wasm memory directly, otherwise you'll have to copy it in afterwards

@najamelan
Copy link
Author

Currently I have a sink/stream working that takes and yields Vec<u8>. The code is clean, documented and I'm looking into the unit testing now. Next I will add a tokio AsyncRead and AsyncWrite onto that. Document and test that and put it on github as proof of concept for review (tonight or tomorrow). After review I'm sure there will be some more cleanup to be done and after that we can discuss adding extra features like a stream of MessageEvents, text messages (currently only does binary), ...

@najamelan
Copy link
Author

Ok, it took a few days longer than I had hoped, but here it is:

https://github.com/najamelan/wasm_websocket_stream

It would be nice if people could review this code. There are review questions in the readme. Please open issues on the repository for review feedback.

@ranile ranile changed the title How about impl AsyncRead/AsyncWrite for web_sys::WebSocket? How about impl AsyncRead/AsyncWrite for WebSocket? Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants