From daa780e2cf2bf606f78c686ac1416786bf85d107 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Sun, 13 Sep 2020 11:52:20 +0200 Subject: [PATCH] fix(WebSocket): no panic on failed connect + handle promise rejection via error event (#7437) --- cli/dts/lib.deno.shared_globals.d.ts | 2 +- cli/ops/websocket.rs | 12 ++++++++---- cli/rt/27_websocket.js | 8 ++++++++ cli/tests/integration_tests.rs | 6 ++++++ cli/tests/websocket_test.ts | 22 +++++++++++++++++++--- 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/cli/dts/lib.deno.shared_globals.d.ts b/cli/dts/lib.deno.shared_globals.d.ts index 8e97fe014a176a..c9423b5f38482e 100644 --- a/cli/dts/lib.deno.shared_globals.d.ts +++ b/cli/dts/lib.deno.shared_globals.d.ts @@ -1489,7 +1489,7 @@ interface WebSocket extends EventTarget { */ readonly extensions: string; onclose: ((this: WebSocket, ev: CloseEvent) => any) | null; - onerror: ((this: WebSocket, ev: Event) => any) | null; + onerror: ((this: WebSocket, ev: Event | ErrorEvent) => any) | null; onmessage: ((this: WebSocket, ev: MessageEvent) => any) | null; onopen: ((this: WebSocket, ev: Event) => any) | null; /** diff --git a/cli/ops/websocket.rs b/cli/ops/websocket.rs index 4a5b83b88854e2..ff360055c819e8 100644 --- a/cli/ops/websocket.rs +++ b/cli/ops/websocket.rs @@ -56,13 +56,12 @@ pub async fn op_ws_create( cli_state.check_net_url(&url::Url::parse(&args.url)?)?; cli_state.global_state.flags.ca_file.clone() }; - let uri: Uri = args.url.parse().unwrap(); + let uri: Uri = args.url.parse()?; let request = Request::builder() .method(Method::GET) .uri(&uri) .header("Sec-WebSocket-Protocol", args.protocols) - .body(()) - .unwrap(); + .body(())?; let domain = &uri.host().unwrap().to_string(); let port = &uri.port_u16().unwrap_or(match uri.scheme_str() { Some("wss") => 443, @@ -100,7 +99,12 @@ pub async fn op_ws_create( }; let (stream, response): (WsStream, Response) = - client_async(request, socket).await.unwrap(); + client_async(request, socket).await.map_err(|err| { + ErrBox::type_error(format!( + "failed to connect to WebSocket: {}", + err.to_string() + )) + })?; let mut state = state.borrow_mut(); let rid = state diff --git a/cli/rt/27_websocket.js b/cli/rt/27_websocket.js index 0b113ebca797c7..fdb5333e3accf4 100644 --- a/cli/rt/27_websocket.js +++ b/cli/rt/27_websocket.js @@ -100,6 +100,14 @@ this.onclose?.(closeEvent); this.dispatchEvent(closeEvent); } + }).catch((err) => { + const event = new ErrorEvent( + "error", + { error: err, message: err.toString() }, + ); + event.target = this; + this.onerror?.(event); + this.dispatchEvent(event); }); } diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 3c2154dffe7e1f..44e56d2463185b 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -403,7 +403,9 @@ fn fmt_stdin_error() { } // Warning: this test requires internet access. +// TODO(#7412): reenable. test is flaky #[test] +#[ignore] fn upgrade_in_tmpdir() { let temp_dir = TempDir::new().unwrap(); let exe_path = temp_dir.path().join("deno"); @@ -423,7 +425,9 @@ fn upgrade_in_tmpdir() { } // Warning: this test requires internet access. +// TODO(#7412): reenable. test is flaky #[test] +#[ignore] fn upgrade_with_space_in_path() { let temp_dir = tempfile::Builder::new() .prefix("directory with spaces") @@ -473,7 +477,9 @@ fn upgrade_with_version_in_tmpdir() { } // Warning: this test requires internet access. +// TODO(#7412): reenable. test is flaky #[test] +#[ignore] fn upgrade_with_out_in_tmpdir() { let temp_dir = TempDir::new().unwrap(); let exe_path = temp_dir.path().join("deno"); diff --git a/cli/tests/websocket_test.ts b/cli/tests/websocket_test.ts index 0fb9af951fedee..93fddf4462cb60 100644 --- a/cli/tests/websocket_test.ts +++ b/cli/tests/websocket_test.ts @@ -1,5 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. import { + assert, assertEquals, assertThrows, createResolvable, @@ -135,18 +136,33 @@ Deno.test("echo string", async () => { }); Deno.test("echo string tls", async () => { - const promise = createResolvable(); + const promise1 = createResolvable(); + const promise2 = createResolvable(); const ws = new WebSocket("wss://localhost:4243"); ws.onerror = (): void => fail(); ws.onopen = (): void => ws.send("foo"); ws.onmessage = (e): void => { assertEquals(e.data, "foo"); ws.close(); + promise1.resolve(); }; ws.onclose = (): void => { - promise.resolve(); + promise2.resolve(); }; - await promise; + await promise1; + await promise2; +}); + +Deno.test("websocket error", async () => { + const promise1 = createResolvable(); + const ws = new WebSocket("wss://localhost:4242"); + ws.onopen = () => fail(); + ws.onerror = (err): void => { + assert(err instanceof ErrorEvent); + assertEquals(err.message, "InvalidData: received corrupt message"); + promise1.resolve(); + }; + await promise1; }); Deno.test("echo blob with binaryType blob", async () => {