Add a websocket transport #84
Conversation
libp2p-swarm/src/transport.rs
Outdated
| Err((second, addr)) => Err((OrTransport(first, second), addr)), | ||
| } | ||
| #[inline] | ||
| fn next_incoming(self) -> Self::Incoming { |
There was a problem hiding this comment.
Why do you forward the addr from first and second and then .map it away immediately?
There was a problem hiding this comment.
It's not the address that is mapped away but a SelectNext object.
libp2p-swarm/src/transport.rs
Outdated
| /// | ||
| /// This function returns the next incoming substream. You are strongly encouraged to call it | ||
| /// if you have a muxed transport. | ||
| pub fn next_incoming(self) -> Box<Future<Item = (C::Output, Multiaddr), Error = IoError> + 'a> |
There was a problem hiding this comment.
We can do this using as fn(_) -> _ and then copying the giant resulting type signature out of the error message, although it's your call as to whether this is worth doing.
There was a problem hiding this comment.
We can't because unfortunately one of the closures captures a value.
libp2p-swarm/src/transport.rs
Outdated
|
|
||
| Box::new(dialed_fut) as _ | ||
| }) | ||
| Box::new(future) as Box<_> |
There was a problem hiding this comment.
Mixed tabs and spaces
libp2p-swarm/src/transport.rs
Outdated
| Err(err) => Ok((Err(err), client_addr)), | ||
| } | ||
| }) | ||
| .map(move |(connection, client_addr)| { |
There was a problem hiding this comment.
Same as above, we can do this without Box
There was a problem hiding this comment.
We also move stuff in the closure here :-/
libp2p-tcp-transport/src/lib.rs
Outdated
| // Pull out a stream of sockets for incoming connections | ||
| listener.incoming().map(|(sock, addr)| { | ||
| println!("incoming tcp stream"); |
libp2p-websocket/src/desktop.rs
Outdated
| OwnedMessage::Text(data) => Ok(Some(data.into_bytes())), | ||
| OwnedMessage::Close(_) => Ok(None), | ||
| // TODO: pings and pongs ; freaking hard | ||
| _ => unimplemented!() |
There was a problem hiding this comment.
Either turn this into a real error with "pings and pongs are unimplemented" as the message or mark this struct as experimental (or something similar), we can't have the server explode if you send a ping.
| .map_err(|err| IoError::new(IoErrorKind::Other, err)) | ||
| .map(|(client, _http_headers)| { | ||
| // Plug our own API on top of the `websockets` API. | ||
| let framed_data = client |
There was a problem hiding this comment.
Mixed tabs and spaces
libp2p-websocket/src/desktop.rs
Outdated
| let dial = inner_dial.into_future().and_then(|connec| { | ||
| // We pass a dummy address to `ClientBuilder` because it is never used anywhere | ||
| // in the negotiation anyway, and we use `async_connect_on` to pass a stream. | ||
| ClientBuilder::new("ws://127.0.0.1:80") |
There was a problem hiding this comment.
Can we fork the WS library to make this cleaner or is it not worth it?
There was a problem hiding this comment.
I would say it's not worth it. Since it's a breaking change they would have to bump their version, leading to more burden.
| //! Listening on a websockets multiaddress isn't supported on emscripten. Dialing a multiaddress | ||
| //! which uses `ws` on top of TCP/IP will automatically use the `XMLHttpRequest` Javascript object. | ||
| //! | ||
| //! ```ignore |
There was a problem hiding this comment.
Why ignore and not no_run?
There was a problem hiding this comment.
This demonstrates the emscripten code, but the doctests are compiled for x86 and hence will fail to compile.
libp2p-websocket/src/browser.rs
Outdated
| impl WsConfig { | ||
| /// Creates a new configuration object for websocket. | ||
| #[inline] | ||
| pub fn new() -> WsConfig { |
There was a problem hiding this comment.
I don't like how the signature for this method changes depending on your platform but otherwise everything acts the same, it seems like it's just asking for confusing error messages when you switch platform and expect everything to work the same. Maybe have BrowserWsConfig and WsConfig?
There was a problem hiding this comment.
It is also true that WsConfig (the desktop one) could be used from inside nodejs or another similar environment, which makes the name BrowserWsConfig more relevant.
|
Should be good now. The ping/pong messages now produce an error and/or close the socket instead of panicking. |
…ct (libp2p#84) * also consider allow self origin when using a published message ids dictionary * make allow_self_origin configurable via the config builder
Based over #81
cc #23
Adds the
libp2p-websocketcrate that allows using websockets as a transport.One things to note is that it provides two completely separate implementations: one for desktop platforms, and one for emscripten.
On emscripten, the transport is a stand-alone, and will handle multiaddresses that look like
/ipN/.../tcp/.../ws. On desktop, the transport needs to be passed an underlying transport (such as aTcpConfig), and will only handle the/wssuffix.The purpose of using websockets is to enable nodes that run in the browser. I have a working example in another branch, which I didn't push because it needs a lot of work.
You can already try to connect two nodes together through websockets by manually providing a dialing or listening address to the
echoexamples. For example, runecho-server /ip4/0.0.0.0/tcp/10333/ws, andecho-dialer /ip4/127.0.0.1/tcp/10333/ws.Note that if you try to connect a node that expects websockets to a node that doesn't expect websockets, or vice-versa, you will get a "cryptic" error (something like "failed to negotiate handshake"). It is not possible to configure the dialer or listener to try websockets and fall back to not-using-websockets if that fails, but I don't think it is a problem.