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

Report error rather than panicking with unreachable! #11

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

gibbz00
Copy link

@gibbz00 gibbz00 commented Jun 14, 2023

The title should be somewhat self-explanatory. Had a project of my crash when reaching the previous unreachable! without any explanation in the error. Turns out the error message was "The operation is insecure.", which is not necessarily a bug of itself: https://stackoverflow.com/questions/11768221/firefox-websocket-security-issue/12042843#12042843

@najamelan
Copy link
Owner

hi, thanks for reporting. Sorry I didn't see your message before. I will look into it tomorrow.

@gibbz00
Copy link
Author

gibbz00 commented Jul 1, 2023

No worries!

@najamelan
Copy link
Owner

This is quite crazy because MDN explicitly states this can only throw one exception:
https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/WebSocket

Is there an easy way to reproduce the error you saw? Which browser was it on?

@najamelan
Copy link
Owner

So I setup an https server locally. Connected to ws protocol (eg, server without ssl). I have network.websocket.allowInsecureFromHTTPS set to false in Firefox, yet the websocket just works, so it seems I can't repro this:

Screenshot from 2023-07-02 12-11-32

@najamelan
Copy link
Owner

najamelan commented Jul 2, 2023

The code I used was very simple:

use ws_stream_wasm::*;
use wasm_bindgen::UnwrapThrowExt;

const URL: &str = "ws://localhost:3212/";

fn main()
{
	let program = async
	{
		let (ws, wsio) = WsMeta::connect( URL, None ).await.expect_throw( "Could not create websocket" );

		assert_eq!( WsState::Open, ws  .ready_state() );
		assert_eq!( WsState::Open, wsio.ready_state() );

		ws.wrapped().close().expect_throw( "close WebSocket" );

		assert_eq!( WsState::Closing, ws  .ready_state() );
		assert_eq!( WsState::Closing, wsio.ready_state() );

		ws.close().await.expect_throw( "close ws" );

		assert_eq!( WsState::Closed, ws  .ready_state() );
		assert_eq!( WsState::Closed, wsio.ready_state() );
	};

	wasm_bindgen_futures::spawn_local(program);
}

@gibbz00
Copy link
Author

gibbz00 commented Jul 8, 2023

Hi again,

Yes, I was also using Firefox. The error happened only when the WS signalling server wasn't hosted locally. Do you want me to prepare an example for you?

@najamelan
Copy link
Owner

If there is any way I can reproduce, that would be great. That way I can have a closer look at the error.

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 this pull request may close these issues.

2 participants