From d57f3806b40d8e760a90413867f4d09713eb59b7 Mon Sep 17 00:00:00 2001 From: Khafra Date: Sun, 23 Apr 2023 11:41:51 -0400 Subject: [PATCH 1/4] websocket: add websocketinit Refs: https://github.com/nodejs/undici/issues/1811#issuecomment-1518940816 --- lib/websocket/connection.js | 5 +-- lib/websocket/websocket.js | 46 ++++++++++++++++++-- test/websocket/websocketinit.js | 75 +++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 test/websocket/websocketinit.js diff --git a/lib/websocket/connection.js b/lib/websocket/connection.js index 09770247e3f..9b7a8e4e8b1 100644 --- a/lib/websocket/connection.js +++ b/lib/websocket/connection.js @@ -13,7 +13,6 @@ const { fireEvent, failWebsocketConnection } = require('./util') const { CloseEvent } = require('./events') const { makeRequest } = require('../fetch/request') const { fetching } = require('../fetch/index') -const { getGlobalDispatcher } = require('../global') const channels = {} channels.open = diagnosticsChannel.channel('undici:websocket:open') @@ -27,7 +26,7 @@ channels.socketError = diagnosticsChannel.channel('undici:websocket:socket_error * @param {import('./websocket').WebSocket} ws * @param {(response: any) => void} onEstablish */ -function establishWebSocketConnection (url, protocols, ws, onEstablish) { +function establishWebSocketConnection (url, protocols, ws, onEstablish, options) { // 1. Let requestURL be a copy of url, with its scheme set to "http", if url’s // scheme is "ws", and to "https" otherwise. const requestURL = url @@ -88,7 +87,7 @@ function establishWebSocketConnection (url, protocols, ws, onEstablish) { const controller = fetching({ request, useParallelQueue: true, - dispatcher: getGlobalDispatcher(), + dispatcher: options.dispatcher, processResponse (response) { // 1. If response is a network error or its status is not 101, // fail the WebSocket connection. diff --git a/lib/websocket/websocket.js b/lib/websocket/websocket.js index 164d24c6f8a..feb000b4b5c 100644 --- a/lib/websocket/websocket.js +++ b/lib/websocket/websocket.js @@ -18,6 +18,7 @@ const { establishWebSocketConnection } = require('./connection') const { WebsocketFrameSend } = require('./frame') const { ByteParser } = require('./receiver') const { kEnumerableProperty, isBlobLike } = require('../core/util') +const { getGlobalDispatcher } = require('../global') const { types } = require('util') let experimentalWarned = false @@ -39,7 +40,7 @@ class WebSocket extends EventTarget { * @param {string} url * @param {string|string[]} protocols */ - constructor (url, protocols = []) { + constructor (url, protocols = [], options = undefined) { super() webidl.argumentLengthCheck(arguments, 1, { header: 'WebSocket constructor' }) @@ -52,7 +53,19 @@ class WebSocket extends EventTarget { } url = webidl.converters.USVString(url) - protocols = webidl.converters['DOMString or sequence'](protocols) + protocols = webidl.converters['DOMString or sequence or WebSocketInit'](protocols) + + // user passed a WebSocketInit as 2nd argument + if (!Array.isArray(protocols) && typeof protocols === 'object') { + options = protocols + protocols = protocols.protocols + } else { + if (options != null && typeof options === 'object') { + options = webidl.converters.WebSocketInit(options) + } else { + options = webidl.converters.WebSocketInit({}) + } + } // 1. Let urlRecord be the result of applying the URL parser to url. let urlRecord @@ -110,7 +123,8 @@ class WebSocket extends EventTarget { urlRecord, protocols, this, - (response) => this.#onConnectionEstablished(response) + (response) => this.#onConnectionEstablished(response), + options ) // Each WebSocket object has an associated ready state, which is a @@ -577,6 +591,32 @@ webidl.converters['DOMString or sequence'] = function (V) { return webidl.converters.DOMString(V) } +// This implements the propsal made in https://github.com/whatwg/websockets/issues/42 +webidl.converters.WebSocketInit = webidl.dictionaryConverter([ + { + key: 'protocols', + converter: webidl.converters['DOMString or sequence'], + get defaultValue () { + return [] + } + }, + { + key: 'dispatcher', + converter: (V) => V, + get defaultValue () { + return getGlobalDispatcher() + } + } +]) + +webidl.converters['DOMString or sequence or WebSocketInit'] = function (V) { + if (webidl.util.Type(V) === 'Object' && !(Symbol.iterator in V)) { + return webidl.converters.WebSocketInit(V) + } + + return webidl.converters['DOMString or sequence'](V) +} + webidl.converters.WebSocketSendData = function (V) { if (webidl.util.Type(V) === 'Object') { if (isBlobLike(V)) { diff --git a/test/websocket/websocketinit.js b/test/websocket/websocketinit.js new file mode 100644 index 00000000000..f3846b3ee03 --- /dev/null +++ b/test/websocket/websocketinit.js @@ -0,0 +1,75 @@ +'use strict' + +const { test } = require('tap') +const { WebSocketServer } = require('ws') +const { WebSocket, Dispatcher, Agent } = require('../..') + +test('WebSocketInit', (t) => { + t.plan(4) + + class WsDispatcher extends Dispatcher { + constructor () { + super() + this.agent = new Agent() + } + + dispatch () { + t.pass() + return this.agent.dispatch(...arguments) + } + } + + t.test('WebSocketInit as 2nd param', (t) => { + t.plan(1) + + const server = new WebSocketServer({ port: 0 }) + + server.on('connection', (ws) => { + ws.send(Buffer.from('hello, world')) + }) + + t.teardown(server.close.bind(server)) + + const ws = new WebSocket(`ws://localhost:${server.address().port}`, { + dispatcher: new WsDispatcher() + }) + + ws.onerror = t.fail + + ws.addEventListener('message', async (event) => { + t.equal(await event.data.text(), 'hello, world') + server.close() + ws.close() + }) + }) + + t.test('WebSocketInit as 3rd param', (t) => { + t.plan(1) + + const server = new WebSocketServer({ port: 0 }) + + server.on('connection', (ws) => { + ws.send(Buffer.from('hello, world')) + }) + + t.teardown(server.close.bind(server)) + + const ws = new WebSocket( + `ws://localhost:${server.address().port}`, + undefined, + { + dispatcher: new WsDispatcher() + } + ) + + console.log(ws) + + ws.onerror = t.fail + + ws.addEventListener('message', async (event) => { + t.equal(await event.data.text(), 'hello, world') + server.close() + ws.close() + }) + }) +}) From 59f593e4028d1a59c2fa61d6b1f38fef4970b022 Mon Sep 17 00:00:00 2001 From: Khafra Date: Sun, 23 Apr 2023 11:47:34 -0400 Subject: [PATCH 2/4] update types --- types/websocket.d.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/types/websocket.d.ts b/types/websocket.d.ts index 7524cbda6c4..97508c0d5b6 100644 --- a/types/websocket.d.ts +++ b/types/websocket.d.ts @@ -10,6 +10,7 @@ import { AddEventListenerOptions, EventListenerOrEventListenerObject } from './patch' +import Dispatcher from './dispatcher' export type BinaryType = 'blob' | 'arraybuffer' @@ -67,7 +68,7 @@ interface WebSocket extends EventTarget { export declare const WebSocket: { prototype: WebSocket - new (url: string | URL, protocols?: string | string[]): WebSocket + new (url: string | URL, protocols?: string | string[] | WebSocketInit, options?: WebSocketInit): WebSocket readonly CLOSED: number readonly CLOSING: number readonly CONNECTING: number @@ -121,3 +122,8 @@ export declare const MessageEvent: { prototype: MessageEvent new(type: string, eventInitDict?: MessageEventInit): MessageEvent } + +interface WebSocketInit { + protocols?: string | string[], + dispatcher?: Dispatcher +} From fd75c3f987b96c3b74aa3282c8ef7f4e1f20089f Mon Sep 17 00:00:00 2001 From: Khafra Date: Sun, 23 Apr 2023 18:11:56 -0400 Subject: [PATCH 3/4] remove 3rd param it's not as backwards compatible as I thought... --- lib/websocket/connection.js | 4 +++- lib/websocket/websocket.js | 20 +++++--------------- test/websocket/websocketinit.js | 32 +------------------------------- types/websocket.d.ts | 2 +- 4 files changed, 10 insertions(+), 48 deletions(-) diff --git a/lib/websocket/connection.js b/lib/websocket/connection.js index 9b7a8e4e8b1..581f268e059 100644 --- a/lib/websocket/connection.js +++ b/lib/websocket/connection.js @@ -13,6 +13,7 @@ const { fireEvent, failWebsocketConnection } = require('./util') const { CloseEvent } = require('./events') const { makeRequest } = require('../fetch/request') const { fetching } = require('../fetch/index') +const { getGlobalDispatcher } = require('../global') const channels = {} channels.open = diagnosticsChannel.channel('undici:websocket:open') @@ -25,6 +26,7 @@ channels.socketError = diagnosticsChannel.channel('undici:websocket:socket_error * @param {string|string[]} protocols * @param {import('./websocket').WebSocket} ws * @param {(response: any) => void} onEstablish + * @param {Partial} options */ function establishWebSocketConnection (url, protocols, ws, onEstablish, options) { // 1. Let requestURL be a copy of url, with its scheme set to "http", if url’s @@ -87,7 +89,7 @@ function establishWebSocketConnection (url, protocols, ws, onEstablish, options) const controller = fetching({ request, useParallelQueue: true, - dispatcher: options.dispatcher, + dispatcher: options.dispatcher ?? getGlobalDispatcher(), processResponse (response) { // 1. If response is a network error or its status is not 101, // fail the WebSocket connection. diff --git a/lib/websocket/websocket.js b/lib/websocket/websocket.js index feb000b4b5c..762f25684ab 100644 --- a/lib/websocket/websocket.js +++ b/lib/websocket/websocket.js @@ -40,7 +40,7 @@ class WebSocket extends EventTarget { * @param {string} url * @param {string|string[]} protocols */ - constructor (url, protocols = [], options = undefined) { + constructor (url, protocols = []) { super() webidl.argumentLengthCheck(arguments, 1, { header: 'WebSocket constructor' }) @@ -52,20 +52,10 @@ class WebSocket extends EventTarget { }) } - url = webidl.converters.USVString(url) - protocols = webidl.converters['DOMString or sequence or WebSocketInit'](protocols) + const options = webidl.converters['DOMString or sequence or WebSocketInit'](protocols) - // user passed a WebSocketInit as 2nd argument - if (!Array.isArray(protocols) && typeof protocols === 'object') { - options = protocols - protocols = protocols.protocols - } else { - if (options != null && typeof options === 'object') { - options = webidl.converters.WebSocketInit(options) - } else { - options = webidl.converters.WebSocketInit({}) - } - } + url = webidl.converters.USVString(url) + protocols = options.protocols // 1. Let urlRecord be the result of applying the URL parser to url. let urlRecord @@ -614,7 +604,7 @@ webidl.converters['DOMString or sequence or WebSocketInit'] = functio return webidl.converters.WebSocketInit(V) } - return webidl.converters['DOMString or sequence'](V) + return { protocols: webidl.converters['DOMString or sequence'](V) } } webidl.converters.WebSocketSendData = function (V) { diff --git a/test/websocket/websocketinit.js b/test/websocket/websocketinit.js index f3846b3ee03..4dda3b48188 100644 --- a/test/websocket/websocketinit.js +++ b/test/websocket/websocketinit.js @@ -5,7 +5,7 @@ const { WebSocketServer } = require('ws') const { WebSocket, Dispatcher, Agent } = require('../..') test('WebSocketInit', (t) => { - t.plan(4) + t.plan(2) class WsDispatcher extends Dispatcher { constructor () { @@ -42,34 +42,4 @@ test('WebSocketInit', (t) => { ws.close() }) }) - - t.test('WebSocketInit as 3rd param', (t) => { - t.plan(1) - - const server = new WebSocketServer({ port: 0 }) - - server.on('connection', (ws) => { - ws.send(Buffer.from('hello, world')) - }) - - t.teardown(server.close.bind(server)) - - const ws = new WebSocket( - `ws://localhost:${server.address().port}`, - undefined, - { - dispatcher: new WsDispatcher() - } - ) - - console.log(ws) - - ws.onerror = t.fail - - ws.addEventListener('message', async (event) => { - t.equal(await event.data.text(), 'hello, world') - server.close() - ws.close() - }) - }) }) diff --git a/types/websocket.d.ts b/types/websocket.d.ts index 97508c0d5b6..578c7602e64 100644 --- a/types/websocket.d.ts +++ b/types/websocket.d.ts @@ -68,7 +68,7 @@ interface WebSocket extends EventTarget { export declare const WebSocket: { prototype: WebSocket - new (url: string | URL, protocols?: string | string[] | WebSocketInit, options?: WebSocketInit): WebSocket + new (url: string | URL, protocols?: string | string[] | WebSocketInit): WebSocket readonly CLOSED: number readonly CLOSING: number readonly CONNECTING: number From c56814d23bc9d90601f29e5752d6a3d1f4a3eeb9 Mon Sep 17 00:00:00 2001 From: Khafra Date: Sun, 23 Apr 2023 18:21:07 -0400 Subject: [PATCH 4/4] update docs --- docs/api/WebSocket.md | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/api/WebSocket.md b/docs/api/WebSocket.md index 639a5333a1c..9d374f4046c 100644 --- a/docs/api/WebSocket.md +++ b/docs/api/WebSocket.md @@ -1,17 +1,40 @@ # Class: WebSocket -> ⚠️ Warning: the WebSocket API is experimental and has known bugs. +> ⚠️ Warning: the WebSocket API is experimental. Extends: [`EventTarget`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget) -The WebSocket object provides a way to manage a WebSocket connection to a server, allowing bidirectional communication. The API follows the [WebSocket spec](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket). +The WebSocket object provides a way to manage a WebSocket connection to a server, allowing bidirectional communication. The API follows the [WebSocket spec](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket) and [RFC 6455](https://datatracker.ietf.org/doc/html/rfc6455). ## `new WebSocket(url[, protocol])` Arguments: * **url** `URL | string` - The url's protocol *must* be `ws` or `wss`. -* **protocol** `string | string[]` (optional) - Subprotocol(s) to request the server use. +* **protocol** `string | string[] | WebSocketInit` (optional) - Subprotocol(s) to request the server use, or a [`Dispatcher`](./Dispatcher.md). + +### Example: + +This example will not work in browsers or other platforms that don't allow passing an object. + +```mjs +import { WebSocket, ProxyAgent } from 'undici' + +const proxyAgent = new ProxyAgent('my.proxy.server') + +const ws = new WebSocket('wss://echo.websocket.events', { + dispatcher: proxyAgent, + protocols: ['echo', 'chat'] +}) +``` + +If you do not need a custom Dispatcher, it's recommended to use the following pattern: + +```mjs +import { WebSocket } from 'undici' + +const ws = new WebSocket('wss://echo.websocket.events', ['echo', 'chat']) +``` ## Read More