Skip to content

Commit 445e9a5

Browse files
committed
Remove Clone impl from WebSocket.
When the WebSocket is used with frameworks, passed down as props, it might be `drop`ed automatically, which closes the WebSocket connection. Initially `Clone` was added so sender and receiver can be in different `spawn_local`s but it turns out that `StreamExt::split` solves that problem very well. See #13 for more information about the issue
1 parent fb35677 commit 445e9a5

File tree

1 file changed

+31
-27
lines changed

1 file changed

+31
-27
lines changed

src/websocket/futures.rs

+31-27
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,9 @@ use web_sys::{Blob, MessageEvent};
5151
/// Wrapper around browser's WebSocket API.
5252
#[allow(missing_debug_implementations)]
5353
#[pin_project(PinnedDrop)]
54-
#[derive(Clone)]
5554
pub struct WebSocket {
5655
ws: web_sys::WebSocket,
57-
sink_wakers: Rc<RefCell<Vec<Waker>>>,
56+
sink_waker: Rc<RefCell<Option<Waker>>>,
5857
#[pin]
5958
message_receiver: Receiver<StreamMessage>,
6059
#[allow(clippy::type_complexity)]
@@ -77,23 +76,22 @@ impl WebSocket {
7776
/// [MDN Documentation](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/WebSocket#exceptions_thrown)
7877
/// to learn more.
7978
pub fn open(url: &str) -> Result<Self, JsError> {
80-
let wakers: Rc<RefCell<Vec<Waker>>> = Rc::new(RefCell::new(vec![]));
79+
let waker: Rc<RefCell<Option<Waker>>> = Rc::new(RefCell::new(None));
8180
let ws = web_sys::WebSocket::new(url).map_err(js_to_js_error)?;
8281

8382
let (sender, receiver) = async_broadcast::broadcast(10);
8483

8584
let open_callback: Closure<dyn FnMut()> = {
86-
let wakers = Rc::clone(&wakers);
85+
let waker = Rc::clone(&waker);
8786
Closure::wrap(Box::new(move || {
88-
for waker in wakers.borrow_mut().drain(..) {
87+
if let Some(waker) = waker.borrow_mut().take() {
8988
waker.wake();
9089
}
9190
}) as Box<dyn FnMut()>)
9291
};
9392

9493
ws.set_onopen(Some(open_callback.as_ref().unchecked_ref()));
95-
// open_callback.forget();
96-
//
94+
9795
let message_callback: Closure<dyn FnMut(MessageEvent)> = {
9896
let sender = sender.clone();
9997
Closure::wrap(Box::new(move |e: MessageEvent| {
@@ -106,7 +104,6 @@ impl WebSocket {
106104
};
107105

108106
ws.set_onmessage(Some(message_callback.as_ref().unchecked_ref()));
109-
// message_callback.forget();
110107

111108
let error_callback: Closure<dyn FnMut(web_sys::ErrorEvent)> = {
112109
let sender = sender.clone();
@@ -123,7 +120,6 @@ impl WebSocket {
123120
};
124121

125122
ws.set_onerror(Some(error_callback.as_ref().unchecked_ref()));
126-
// error_callback.forget();
127123

128124
let close_callback: Closure<dyn FnMut(web_sys::CloseEvent)> = {
129125
Closure::wrap(Box::new(move |e: web_sys::CloseEvent| {
@@ -144,11 +140,10 @@ impl WebSocket {
144140
};
145141

146142
ws.set_onerror(Some(close_callback.as_ref().unchecked_ref()));
147-
// close_callback.forget();
148143

149144
Ok(Self {
150145
ws,
151-
sink_wakers: wakers,
146+
sink_waker: waker,
152147
message_receiver: receiver,
153148
closures: Rc::new((
154149
open_callback,
@@ -163,9 +158,6 @@ impl WebSocket {
163158
///
164159
/// See the [MDN Documentation](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/close#parameters)
165160
/// to learn about parameters passed to this function and when it can return an `Err(_)`
166-
///
167-
/// **Note**: If *only one* of the instances of websocket is closed, the entire connection closes.
168-
/// This is unlikely to happen in real-world as [`wasm_bindgen_futures::spawn_local`] requires `'static`.
169161
pub fn close(self, code: Option<u16>, reason: Option<&str>) -> Result<(), JsError> {
170162
let result = match (code, reason) {
171163
(None, None) => self.ws.close(),
@@ -240,7 +232,7 @@ impl Sink<Message> for WebSocket {
240232
fn poll_ready(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
241233
let ready_state = self.ws.ready_state();
242234
if ready_state == 0 {
243-
self.sink_wakers.borrow_mut().push(cx.waker().clone());
235+
*self.sink_waker.borrow_mut() = Some(cx.waker().clone());
244236
Poll::Pending
245237
} else {
246238
Poll::Ready(Ok(()))
@@ -298,23 +290,35 @@ mod tests {
298290
use super::*;
299291
use futures::{SinkExt, StreamExt};
300292
use wasm_bindgen_test::*;
293+
use wasm_bindgen_futures::spawn_local;
301294

302295
wasm_bindgen_test_configure!(run_in_browser);
303296

304297
const ECHO_SERVER_URL: &str = env!("ECHO_SERVER_URL");
305298

306299
#[wasm_bindgen_test]
307-
async fn websocket_works() {
308-
let mut ws = WebSocket::open(ECHO_SERVER_URL).unwrap();
309-
310-
ws.send(Message::Text("test".to_string())).await.unwrap();
311-
312-
// ignore first message
313-
// the echo-server used sends it's info in the first message
314-
let _ = ws.next().await;
315-
assert_eq!(
316-
ws.next().await.unwrap().unwrap(),
317-
Message::Text("test".to_string())
318-
)
300+
fn websocket_works() {
301+
let ws = WebSocket::open(ECHO_SERVER_URL).unwrap();
302+
let (mut sender, mut receiver) = ws.split();
303+
304+
spawn_local(async move {
305+
sender.send(Message::Text(String::from("test 1"))).await.unwrap();
306+
sender.send(Message::Text(String::from("test 2"))).await.unwrap();
307+
});
308+
309+
spawn_local(async move {
310+
// ignore first message
311+
// the echo-server used sends it's info in the first message
312+
// let _ = ws.next().await;
313+
314+
assert_eq!(
315+
receiver.next().await.unwrap().unwrap(),
316+
Message::Text("test 1".to_string())
317+
);
318+
assert_eq!(
319+
receiver.next().await.unwrap().unwrap(),
320+
Message::Text("test 2".to_string())
321+
);
322+
});
319323
}
320324
}

0 commit comments

Comments
 (0)