Skip to content

Commit

Permalink
fix(WebSocket): no panic on failed connect + handle promise rejection…
Browse files Browse the repository at this point in the history
… via error event (#7437)
  • Loading branch information
lucacasonato authored Sep 13, 2020
1 parent 82d0f7e commit daa780e
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cli/dts/lib.deno.shared_globals.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand Down
12 changes: 8 additions & 4 deletions cli/ops/websocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions cli/rt/27_websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

Expand Down
6 changes: 6 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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")
Expand Down Expand Up @@ -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");
Expand Down
22 changes: 19 additions & 3 deletions cli/tests/websocket_test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
import {
assert,
assertEquals,
assertThrows,
createResolvable,
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit daa780e

Please sign in to comment.