From 579b243e89ac7dc58233f9844ef70817364ecf52 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Tue, 28 May 2024 19:31:36 +0200 Subject: [PATCH] feat: add the ability to test all transports When setting the `tryAllTransports` option to `true`, if the first transport (usually, HTTP long-polling) fails, then the other transports will be tested too. This is useful in two cases: > when HTTP long-polling is disabled on the server, or if CORS fails Related: - https://github.com/socketio/engine.io-client/issues/575 - https://github.com/socketio/socket.io-client/issues/1448 > when WebSocket is tested first (`transports: ["websocket", "polling"]) Related: - https://github.com/socketio/engine.io-client/issues/714 - https://github.com/socketio/socket.io-client/issues/1599 The only potential downside is that the connection attempt could take more time in case of failure, as there have been reports of WebSocket connection errors taking several seconds before being detected (that's one reason for using HTTP long-polling first). That's why the option defaults to `false` for now. --- lib/socket.ts | 24 ++++++++++++++ lib/transports/websocket.ts | 19 +++-------- lib/transports/webtransport.ts | 20 +++++++----- test/socket.js | 60 ++++++++++++++++++++++++++++++++++ test/support/hooks.js | 6 ++++ 5 files changed, 106 insertions(+), 23 deletions(-) diff --git a/lib/socket.ts b/lib/socket.ts index 6460ea46e..13f6a9ef2 100644 --- a/lib/socket.ts +++ b/lib/socket.ts @@ -83,6 +83,19 @@ export interface SocketOptions { */ transports?: string[]; + /** + * Whether all the transports should be tested, instead of just the first one. + * + * If set to `true`, the client will first try to connect with HTTP long-polling, and then with WebSocket in case of + * failure, and finally with WebTransport if the previous attempts have failed. + * + * If set to `false` (default), if the connection with HTTP long-polling fails, then the client will not test the + * other transports and will abort the connection. + * + * @default false + */ + tryAllTransports?: boolean; + /** * If true and if the previous websocket connection to the server succeeded, * the connection attempt will bypass the normal upgrade process and will @@ -916,6 +929,17 @@ export class Socket extends Emitter< private onError(err: Error) { debug("socket error %j", err); Socket.priorWebsocketSuccess = false; + + if ( + this.opts.tryAllTransports && + this.transports.length > 1 && + this.readyState === "opening" + ) { + debug("trying next transport"); + this.transports.shift(); + return this.open(); + } + this.emitReserved("error", err); this.onClose("transport error", err); } diff --git a/lib/transports/websocket.ts b/lib/transports/websocket.ts index 45bc47d1e..d2b35e789 100644 --- a/lib/transports/websocket.ts +++ b/lib/transports/websocket.ts @@ -17,6 +17,10 @@ const isReactNative = typeof navigator.product === "string" && navigator.product.toLowerCase() === "reactnative"; +/** + * @see https://developer.mozilla.org/en-US/docs/Web/API/WebSocket + * @see https://caniuse.com/mdn-api_websocket + */ export class WS extends Transport { private ws: any; @@ -37,11 +41,6 @@ export class WS extends Transport { } override doOpen() { - if (!this.check()) { - // let probe timeout - return; - } - const uri = this.uri(); const protocols = this.opts.protocols; @@ -189,14 +188,4 @@ export class WS extends Transport { return this.createUri(schema, query); } - - /** - * Feature detection for WebSocket. - * - * @return {Boolean} whether this transport is available. - * @private - */ - private check() { - return !!WebSocket; - } } diff --git a/lib/transports/webtransport.ts b/lib/transports/webtransport.ts index 7996a2f05..78ae74934 100644 --- a/lib/transports/webtransport.ts +++ b/lib/transports/webtransport.ts @@ -9,6 +9,10 @@ import debugModule from "debug"; // debug() const debug = debugModule("engine.io-client:webtransport"); // debug() +/** + * @see https://developer.mozilla.org/en-US/docs/Web/API/WebTransport + * @see https://caniuse.com/webtransport + */ export class WT extends Transport { private transport: any; private writer: any; @@ -18,15 +22,15 @@ export class WT extends Transport { } protected doOpen() { - // @ts-ignore - if (typeof WebTransport !== "function") { - return; + try { + // @ts-ignore + this.transport = new WebTransport( + this.createUri("https"), + this.opts.transportOptions[this.name] + ); + } catch (err) { + return this.emitReserved("error", err); } - // @ts-ignore - this.transport = new WebTransport( - this.createUri("https"), - this.opts.transportOptions[this.name] - ); this.transport.closed .then(() => { diff --git a/test/socket.js b/test/socket.js index 5bc60f14a..d0e1e2745 100644 --- a/test/socket.js +++ b/test/socket.js @@ -37,6 +37,66 @@ describe("Socket", function () { }); }); + it("should connect with the 2nd transport if tryAllTransports is `true` (polling)", (done) => { + const socket = new Socket({ + transports: ["websocket", "polling"], + transportOptions: { + websocket: { + query: { + deny: 1, + }, + }, + }, + tryAllTransports: true, + }); + + socket.on("open", () => { + expect(socket.transport.name).to.eql("polling"); + socket.close(); + done(); + }); + }); + + it("should connect with the 2nd transport if tryAllTransports is `true` (websocket)", (done) => { + const socket = new Socket({ + transports: ["polling", "websocket"], + transportOptions: { + polling: { + query: { + deny: 1, + }, + }, + }, + tryAllTransports: true, + }); + + socket.on("open", () => { + expect(socket.transport.name).to.eql("websocket"); + socket.close(); + done(); + }); + }); + + it("should not connect with the 2nd transport if tryAllTransports is `false`", (done) => { + const socket = new Socket({ + transports: ["polling", "websocket"], + transportOptions: { + polling: { + query: { + deny: 1, + }, + }, + }, + }); + + socket.on("error", (err) => { + expect(err.message).to.eql( + useFetch ? "fetch read error" : "xhr poll error" + ); + done(); + }); + }); + describe("fake timers", function () { before(function () { if (isIE11 || isAndroid || isEdge || isIPad) { diff --git a/test/support/hooks.js b/test/support/hooks.js index 75281ef53..75623cc30 100644 --- a/test/support/hooks.js +++ b/test/support/hooks.js @@ -19,6 +19,12 @@ exports.mochaHooks = { engine = attach(httpServer, { pingInterval: 500, maxHttpBufferSize: 100, + allowRequest: (req, fn) => { + const denyRequest = new URL(`http://${req.url}`).searchParams.has( + "deny" + ); + fn(null, !denyRequest); + }, }); rollup(rollupConfig).then(async (bundle) => {