Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: najamelan/ws_stream_wasm
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 0.7.3
Choose a base ref
...
head repository: najamelan/ws_stream_wasm
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 0.7.4
Choose a head ref
  • 11 commits
  • 11 files changed
  • 3 contributors

Commits on Oct 29, 2022

  1. Update tokio-util.

    najamelan committed Oct 29, 2022
    Copy the full SHA
    70f2528 View commit details
  2. Add liberapay tag.

    najamelan committed Oct 29, 2022
    Copy the full SHA
    7353294 View commit details
  3. Update send_wrapper.

    najamelan committed Oct 29, 2022
    Copy the full SHA
    abc475d View commit details
  4. Copy the full SHA
    029a0cd View commit details

Commits on Jan 13, 2023

  1. Fix a bug where the onerror callback could try to be called when it…

    … had
    
    already been destroyed.
    
    If there was an error establishing a WebSocket connection at an `await` point
    and after the point at which the `onerror` etc callbacks had been set but before
    the `WsStream` had been constructed then the `onerror` callback would try to be
    called but it would have already been destroyed because of the future being
    cancelled.
    
    This could happen if, for example, the initial attempt to connect to a WebSocket
    server timed out and we never observed an event at this `await` point:
    
    ```rust
    let mut evts = pharos.observe( Self::OPEN_CLOSE.into() ).await
    ```
    
    To fix this we introduce a guard that is only active across the `await` points
    in the `WsMeta::connect` function. It is temporarily responsible for
    unregistering the callbacks in the case that the future is cancelled before the
    `WsStream` is constructed.
    
    The error that you'd otherwise run into would present itself like this in the
    logs:
    
    ```
    ERROR: Error: closure invoked recursively or destroyed already
        at imports.wbg.__wbindgen_throw (http://localhost:61367/package/web/ditto.es6.js:1:70261)
        at wasm_bindgen::throw_str::hf6afd94675c6db0f (http://localhost:61367/package/web/ditto.wasm:wasm-function[119306]:0x3327743)
        at <dyn core::ops::function::FnMut<()>+Output = R as wasm_bindgen::closure::WasmClosure>::describe::invoke::h2944e7516b507b14 (http://localhost:61367/package/web/ditto.wasm:wasm-function[49343]:0x2c601d8)
        at __wbg_adapter_36 (http://localhost:61367/package/web/ditto.es6.js:1:12851)
        at WebSocket.real (http://localhost:61367/package/web/ditto.es6.js:1:11649)
    ```
    
    Co-Authored-By: Daniel Henry-Mantilla <daniel.henry.mantilla@gmail.com>
    hamchapman and danielhenrymantilla committed Jan 13, 2023
    Copy the full SHA
    ee876f8 View commit details

Commits on Jan 29, 2023

  1. Copy the full SHA
    fae6529 View commit details
  2. Clippy warnings.

    najamelan committed Jan 29, 2023
    Copy the full SHA
    0ea8699 View commit details
  3. Merge pull request #9 from hamchapman/hc/closure-drop-on-future-cance…

    …l-fix
    
    Fix a bug where the `onerror` callback could try to be called when it had already been destroyed
    najamelan authored Jan 29, 2023
    Copy the full SHA
    327ae0a View commit details
  4. Update github actions.

    najamelan committed Jan 29, 2023
    Copy the full SHA
    343c22a View commit details
  5. Copy the full SHA
    4035d46 View commit details
  6. Merge branch 'dev' into release for 0.7.4

    * dev:
      Changelog and version number.
      Update github actions.
      Clippy warnings.
      On future drop while connecting, close ws and log warning.
      Fix a bug where the `onerror` callback could try to be called when it had already been destroyed.
      Use cargo deny github action.
      Update send_wrapper.
      Add liberapay tag.
      Update tokio-util.
    najamelan committed Jan 29, 2023
    Copy the full SHA
    f68f4d5 View commit details
Showing with 78 additions and 50 deletions.
  1. +1 −0 .github/funding.yml
  2. +5 −6 .github/workflows/ci.yml
  3. +15 −0 CHANGELOG.md
  4. +5 −5 Cargo.toml
  5. +5 −5 Cargo.yml
  6. +0 −14 ci/deny.bash
  7. +5 −2 src/ws_event.rs
  8. +31 −4 src/ws_meta.rs
  9. +1 −1 src/ws_stream.rs
  10. +1 −1 tests/ws_meta.rs
  11. +9 −12 tests/ws_stream.rs
1 change: 1 addition & 0 deletions .github/funding.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
liberapay: najamelan
11 changes: 5 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ jobs:


- name: Checkout crate
uses: actions/checkout@v2
uses: actions/checkout@v3


- name: Checkout server
@@ -62,11 +62,7 @@ jobs:


- name: Checkout crate
uses: actions/checkout@v2


- name: Check deny
run: bash ci/deny.bash
uses: actions/checkout@v3


- name: Run clippy
@@ -84,5 +80,8 @@ jobs:
- name: Run tests
run : bash ci/test.bash

- name: Run cargo-deny
uses: EmbarkStudios/cargo-deny-action@v1



15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -5,6 +5,21 @@
[Unreleased]: https://github.com/najamelan/ws_stream_wasm/compare/release...dev


## [0.7.4] - 2023-01-29

[0.7.4]: https://github.com/najamelan/ws_stream_wasm/compare/0.7.3...0.7.4

### Fixed

- When the `WsMeta::connect` future is dropped, close websocket and unregister callbacks.
This avoids some ugly error messages in the console. Thanks to @hamchapman for discovering
and solving the issue and @danielhenrymantilla for reviewing the solution.

### Updated
- tokio-util to 0.7 (dev-dependency)
- send_wrapper to 0.6


## [0.7.3] - 2021-06-11

[0.7.3]: https://github.com/najamelan/ws_stream_wasm/compare/0.7.2...0.7.3
10 changes: 5 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@ rustc_version = "^0.4"

[dependencies]
js-sys = "^0.3"
send_wrapper = "^0.5"
log = "^0.4"
send_wrapper = "^0.6"
thiserror = "^1"
wasm-bindgen = "^0.2"
wasm-bindgen-futures = "^0.4"
@@ -35,7 +36,6 @@ version = "^0.3"
bytes = "^1"
console_error_panic_hook = "^0.1"
console_log = "^0.2"
log = "^0.4"
rand = "^0.8"
rand_xoshiro = "^0.6"
serde_cbor = "^0.11"
@@ -60,12 +60,12 @@ version = "^1"
version = "^1"

[dev-dependencies.tokio-serde-cbor]
version = "^0.6"
version = "^0.7"

[dev-dependencies.tokio-util]
default-features = false
features = ["codec"]
version = "^0.6"
version = "^0.7"

[features]
tokio_io = ["async_io_stream/tokio_io"]
@@ -83,7 +83,7 @@ license = "Unlicense"
name = "ws_stream_wasm"
readme = "README.md"
repository = "https://github.com/najamelan/ws_stream_wasm"
version = "0.7.3"
version = "0.7.4"

[package.metadata]
[package.metadata.docs]
10 changes: 5 additions & 5 deletions Cargo.yml
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@ package:
# - `git tag x.x.x` with version number.
# - `git push && git push --tags`
#
version : 0.7.3
version : 0.7.4
name : ws_stream_wasm
edition : '2018'
authors : [ Naja Melan <najamelan@autistici.org> ]
@@ -84,9 +84,10 @@ dependencies:
#
js-sys : ^0.3
thiserror : ^1
send_wrapper : ^0.5
send_wrapper : ^0.6
wasm-bindgen : ^0.2
wasm-bindgen-futures : ^0.4
log : ^0.4


dev-dependencies:
@@ -98,16 +99,15 @@ dev-dependencies:
console_log : ^0.2
# flexi_logger : ^0.16
asynchronous-codec : { version: ^0.6 }
log : ^0.4
# pretty_assertions : ^0.6
rand : ^0.8
rand_xoshiro : ^0.6
getrandom : { version: ^0.2, features: [js] }
serde : { version: ^1, features: [ derive ] }
serde_cbor : ^0.11
tokio : { version: ^1 }
tokio-serde-cbor : { version: ^0.6 }
tokio-util : { version: ^0.6, default-features: false, features: [codec] }
tokio-serde-cbor : { version: ^0.7 }
tokio-util : { version: ^0.7, default-features: false, features: [codec] }
wasm-bindgen-test : ^0.3


14 changes: 0 additions & 14 deletions ci/deny.bash

This file was deleted.

7 changes: 5 additions & 2 deletions src/ws_event.rs
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ use crate::{ import::*, WsErr };
/// ws_stream_wasm :: * ,
/// pharos :: * ,
/// wasm_bindgen :: UnwrapThrowExt ,
/// wasm_bindgen_futures :: futures_0_3::spawn_local ,
/// wasm_bindgen_futures :: spawn_local ,
/// futures :: stream::StreamExt ,
///};
///
@@ -21,7 +21,10 @@ use crate::{ import::*, WsErr };
///
/// // The Filter type comes from the pharos crate.
/// //
/// let mut evts = ws.observe( Filter::Pointer( WsEvent::is_closed ).into() );
/// let mut evts = ws.observe( Filter::Pointer( WsEvent::is_closed ).into() )
/// .await
/// .expect("observe ws_meta")
/// ;
///
/// ws.close().await;
///
35 changes: 31 additions & 4 deletions src/ws_meta.rs
Original file line number Diff line number Diff line change
@@ -92,10 +92,10 @@ impl WsMeta
// Create our pharos.
//
let mut pharos = SharedPharos::default();
let ph1 = pharos.clone();
let ph2 = pharos.clone();
let ph3 = pharos.clone();
let ph4 = pharos.clone();
let ph1 = pharos.clone();
let ph2 = pharos.clone();
let ph3 = pharos.clone();
let ph4 = pharos.clone();


// Setup our event listeners
@@ -145,7 +145,30 @@ impl WsMeta
ws.set_onclose( Some( on_close.as_ref().unchecked_ref() ));
ws.set_onerror( Some( on_error.as_ref().unchecked_ref() ));

// In case of future task cancellation the current task may be interrupted at an await, therefore not reaching
// the `WsStream` construction, whose `Drop` glue would have been responsible for unregistering the callbacks.
// We therefore use a guard to be responsible for unregistering the callbacks until the `WsStream` is
// constructed.
//
let guard =
{
struct Guard<'lt> { ws: &'lt WebSocket }

impl Drop for Guard<'_>
{
fn drop(&mut self)
{
self.ws.set_onopen(None);
self.ws.set_onclose(None);
self.ws.set_onerror(None);
self.ws.close().unwrap_throw(); // cannot throw without code and reason.

log::warn!( "WsMeta::connect future was dropped while connecting to: {}.", self.ws.url() );
}
}

Guard { ws: &ws }
};

// Listen to the events to figure out whether the connection opens successfully. We don't want to deal with
// the error event. Either a close event happens, in which case we want to recover the CloseEvent to return it
@@ -163,6 +186,10 @@ impl WsMeta
return Err( WsErr::ConnectionFailed{ event: evt } )
}

// We have now passed all the `await` points in this function and so the `WsStream` construction is guaranteed
// so we let it take over the responsibility of unregistering the callbacks by disabling our guard.
//
std::mem::forget(guard);

// We don't handle Blob's
//
2 changes: 1 addition & 1 deletion src/ws_stream.rs
Original file line number Diff line number Diff line change
@@ -385,7 +385,7 @@ impl Sink<WsMessage> for WsStream
}


let _ = ready!( self.closer.as_mut().unwrap().as_mut().poll(cx) );
ready!( self.closer.as_mut().unwrap().as_mut().poll(cx) );

Ok(()).into()
}
2 changes: 1 addition & 1 deletion tests/ws_meta.rs
Original file line number Diff line number Diff line change
@@ -377,7 +377,7 @@ async fn debug()

let (ws, _wsio) = WsMeta::connect( URL, None ).await.expect_throw( "Could not create websocket" );

assert_eq!( format!( "WsMeta for connection: {}", URL ), format!( "{:?}", ws ) );
assert_eq!( format!( "WsMeta for connection: {URL}" ), format!( "{ws:?}" ) );

ws.close().await.expect_throw( "close" );
}
21 changes: 9 additions & 12 deletions tests/ws_stream.rs
Original file line number Diff line number Diff line change
@@ -16,14 +16,14 @@ wasm_bindgen_test_configure!(run_in_browser);
//
use
{
futures::prelude :: * ,
log :: * ,
pharos :: * ,
futures::prelude :: * ,
log :: * ,
pharos :: * ,
std::marker :: PhantomData ,
wasm_bindgen::prelude :: * ,
wasm_bindgen::prelude :: * ,
wasm_bindgen_futures :: spawn_local ,
wasm_bindgen_test :: * ,
ws_stream_wasm :: * ,
wasm_bindgen_test :: * ,
ws_stream_wasm :: * ,
};


@@ -33,9 +33,6 @@ const URL_TT: &str = "ws://127.0.0.1:3312/";






// Verify that both WsStream and WsMeta are Send for now. The browser API's are not Send,
// and this is not meant to be send accross threads. However some API's need to require
// Send (eg async that can be spawned on a thread pool). However on wasm you can spawn them
@@ -62,7 +59,7 @@ async fn round_trip_text()
{
let _ = console_log::init_with_level( Level::Trace );

info!( "starting test: round_trip" );
info!( "starting test: round_trip_text" );

let (_ws, mut wsio) = WsMeta::connect( URL_TT, None ).await.expect_throw( "Could not create websocket" );
let message = "Hello from browser".to_string();
@@ -89,7 +86,7 @@ async fn round_trip_binary()
{
let _ = console_log::init_with_level( Level::Trace );

info!( "starting test: round_trip" );
info!( "starting test: round_trip_binary" );

let (_ws, mut wsio) = WsMeta::connect( URL, None ).await.expect_throw( "Could not create websocket" );
let message = b"Hello from browser".to_vec();
@@ -239,7 +236,7 @@ async fn debug()

let (_ws, mut wsio) = WsMeta::connect( URL, None ).await.expect_throw( "Could not create websocket" );

assert_eq!( format!( "WsStream for connection: {}", URL ), format!( "{:?}", wsio ) );
assert_eq!( format!( "WsStream for connection: {URL}" ), format!( "{wsio:?}" ) );

wsio.close().await.expect_throw( "close" );
}