From 531754c35497568aa2f19179344eb9e205c9a4b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 25 Apr 2023 13:53:06 +0200 Subject: [PATCH] refactor(ext/websocket): use specialized ops (#18819) Instead of relying on `op_ws_send` to send different kinds of messages, use specialized ops everywhere. --- cli/js/40_testing.js | 5 ++++- ext/websocket/01_websocket.js | 8 +------ ext/websocket/02_websocketstream.js | 14 +++--------- ext/websocket/lib.rs | 33 ++++++++++++++++------------- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/cli/js/40_testing.js b/cli/js/40_testing.js index a0dcaf4991..1464483563 100644 --- a/cli/js/40_testing.js +++ b/cli/js/40_testing.js @@ -128,7 +128,10 @@ const OP_DETAILS = { "op_ws_close": ["close a WebSocket", "awaiting until the `close` event is emitted on a `WebSocket`, or the `WebSocketStream#closed` promise resolves"], "op_ws_create": ["create a WebSocket", "awaiting until the `open` event is emitted on a `WebSocket`, or the result of a `WebSocketStream#connection` promise"], "op_ws_next_event": ["receive the next message on a WebSocket", "closing a `WebSocket` or `WebSocketStream`"], - "op_ws_send": ["send a message on a WebSocket", "closing a `WebSocket` or `WebSocketStream`"], + "op_ws_send_text": ["send a message on a WebSocket", "closing a `WebSocket` or `WebSocketStream`"], + "op_ws_send_binary": ["send a message on a WebSocket", "closing a `WebSocket` or `WebSocketStream`"], + "op_ws_send_ping": ["send a message on a WebSocket", "closing a `WebSocket` or `WebSocketStream`"], + "op_ws_send_pong": ["send a message on a WebSocket", "closing a `WebSocket` or `WebSocketStream`"], }; // Wrap test function in additional assertion that makes sure diff --git a/ext/websocket/01_websocket.js b/ext/websocket/01_websocket.js index 60378b6758..1b7a45ce0b 100644 --- a/ext/websocket/01_websocket.js +++ b/ext/websocket/01_websocket.js @@ -534,13 +534,7 @@ class WebSocket extends EventTarget { clearTimeout(this[_idleTimeoutTimeout]); this[_idleTimeoutTimeout] = setTimeout(async () => { if (this[_readyState] === OPEN) { - await core.opAsync( - "op_ws_send", - this[_rid], - { - kind: "ping", - }, - ); + await core.opAsync("op_ws_send_ping", this[_rid]); this[_idleTimeoutTimeout] = setTimeout(async () => { if (this[_readyState] === OPEN) { this[_readyState] = CLOSING; diff --git a/ext/websocket/02_websocketstream.js b/ext/websocket/02_websocketstream.js index 0d01e62eea..f545d7a99b 100644 --- a/ext/websocket/02_websocketstream.js +++ b/ext/websocket/02_websocketstream.js @@ -207,17 +207,11 @@ class WebSocketStream { const writable = new WritableStream({ write: async (chunk) => { if (typeof chunk === "string") { - await core.opAsync("op_ws_send", this[_rid], { - kind: "text", - value: chunk, - }); + await core.opAsync2("op_ws_send_text", this[_rid], chunk); } else if ( ObjectPrototypeIsPrototypeOf(Uint8ArrayPrototype, chunk) ) { - await core.opAsync("op_ws_send", this[_rid], { - kind: "binary", - value: chunk, - }, chunk); + await core.opAsync2("op_ws_send_binary", this[_rid], chunk); } else { throw new TypeError( "A chunk may only be either a string or an Uint8Array", @@ -265,9 +259,7 @@ class WebSocketStream { } case 3: { /* ping */ - await core.opAsync("op_ws_send", this[_rid], { - kind: "pong", - }); + await core.opAsync("op_ws_send_pong", this[_rid]); await pull(controller); break; } diff --git a/ext/websocket/lib.rs b/ext/websocket/lib.rs index 07cddc85bb..74898a471f 100644 --- a/ext/websocket/lib.rs +++ b/ext/websocket/lib.rs @@ -406,27 +406,29 @@ pub async fn op_ws_send_text( } #[op] -pub async fn op_ws_send( +pub async fn op_ws_send_ping( state: Rc>, rid: ResourceId, - value: SendValue, ) -> Result<(), AnyError> { - let msg = match value { - SendValue::Text(text) => { - Frame::new(true, OpCode::Text, None, text.into_bytes()) - } - SendValue::Binary(buf) => { - Frame::new(true, OpCode::Binary, None, buf.to_vec()) - } - SendValue::Pong => Frame::new(true, OpCode::Pong, None, vec![]), - SendValue::Ping => Frame::new(true, OpCode::Ping, None, vec![]), - }; - let resource = state .borrow_mut() .resource_table .get::(rid)?; - resource.write_frame(msg).await + resource + .write_frame(Frame::new(true, OpCode::Ping, None, vec![])) + .await +} + +#[op] +pub async fn op_ws_send_pong( + state: Rc>, + rid: ResourceId, +) -> Result<(), AnyError> { + let resource = state + .borrow_mut() + .resource_table + .get::(rid)?; + resource.write_frame(Frame::pong(vec![])).await } #[op(deferred)] @@ -521,11 +523,12 @@ deno_core::extension!(deno_websocket, ops = [ op_ws_check_permission_and_cancel_handle

, op_ws_create

, - op_ws_send, op_ws_close, op_ws_next_event, op_ws_send_binary, op_ws_send_text, + op_ws_send_ping, + op_ws_send_pong, op_ws_server_create, ], esm = [ "01_websocket.js", "02_websocketstream.js" ],