fix(wsdriver): provide connection error info

src/browser/wsdriver/request.ts

@@ -63,7 +63,7 @@ export const constructWsDriverRequest = async (
         throw new Error("Not session-related request");
     }
 
-    const command = url.pathname.slice(commandStartIdx + connectionOptions.sessionPrefix.length);
+    const command = decodeURIComponent(url.pathname.slice(commandStartIdx + connectionOptions.sessionPrefix.length));
     const bodyPayload = requestOptions.json ? Buffer.from(JSON.stringify(requestOptions.json)) : Buffer.alloc(0);
     const shouldCompress =
         connectionOptions.compressionType !== WsDriverCompression.None &&

src/ws-connection/error.ts

@@ -26,13 +26,17 @@ export abstract class WsError extends Error {
 }
 
 export class WsConnectionEstablishmentError extends WsError {
-    constructor({ message, requestId }: { message: string; requestId?: number }) {
-        super({ message, code: WS_ERROR_CODE.CONNECTION_ESTABLISHMENT, requestId });
+    constructor({ message, requestId, statusCode }: { message: string; requestId?: number; statusCode?: number }) {
+        super({ message, code: statusCode || WS_ERROR_CODE.CONNECTION_ESTABLISHMENT, requestId });
 
         this.name = this.constructor.name;
     }
 
     isRetryable(): boolean {
+        if (this.code && this.code >= 400 && this.code < 500) {
+            return false;
+        }
+
         return true;
     }
 }

src/ws-connection/index.ts

@@ -1,5 +1,6 @@
 /* eslint-disable new-cap */
 import { IncomingMessage } from "node:http";
+import { text as consumeText } from "node:stream/consumers";
 import { WebSocket, type RawData } from "ws";
 import {
     WsTimeoutError,
@@ -146,6 +147,19 @@ export class WsConnection<
                     done(ws);
                 };
 
+                const onUnexpectedResponse = async (ws: WebSocket, res: IncomingMessage): Promise<void> => {
+                    closeWsConnection(ws);
+
+                    const reason = await consumeText(res).catch(() => "Unknown reason");
+
+                    done(
+                        new this._errors.ConnectionEstablishment({
+                            message: `Couldn't establish WS connection to "${endpoint}"\n\tReason: ${reason}`,
+                            statusCode: res.statusCode,
+                        }),
+                    );
+                };
+
                 const onError = (error: unknown): void => {
                     closeWsConnection(ws);
                     done(
@@ -169,6 +183,7 @@ export class WsConnection<
                 };
 
                 ws.on("open", onOpen);
+                ws.on("unexpected-response", onUnexpectedResponse);
                 ws.on("error", onError);
                 ws.on("close", onClose);
                 ws.on("upgrade", onUpgrade);
@@ -183,6 +198,7 @@ export class WsConnection<
                     isSettled = true;
                     clearTimeout(timeoutId);
                     ws.off("open", onOpen);
+                    ws.off("unexpected-response", onUnexpectedResponse);
                     ws.off("error", onError);
                     ws.off("close", onClose);
                     ws.off("upgrade", onUpgrade);
@@ -276,7 +292,7 @@ export class WsConnection<
                         throw result;
                     }
 
-                    this._debugFn(`⟳ ${result.message}; retries left: ${retriesLeft}`);
+                    this._debugFn(`⟳ ${result.message}; retries left: ${retriesLeft}; endpoint: "${this._endpoint}"`);
 
                     // Intentionally avoiding wait after timeout
                     if (result instanceof WsError && !(result instanceof WsTimeoutError)) {
@@ -383,7 +399,9 @@ export class WsConnection<
 
     provideResponseFor(requestId: number, data: ResponseMessageType | WsError): void {
         if (!this._pendingRequests[requestId]) {
-            this._debugFn(`! Received response to request ${requestId}, which is probably timed out already`);
+            this._debugFn(
+                `! Received response to request ${requestId}, which is probably timed out already; endpoint: "${this._endpoint}"`,
+            );
             return;
         }
 
@@ -508,7 +526,7 @@ export class WsConnection<
             if (isWaitingForPong && this._isWebSocketActive(ws)) {
                 isWaitingForPong = false;
 
-                this._debugFn("< PONG");
+                this._debugFn(`< PONG; endpoint: "${this._endpoint}"`);
 
                 clearTimeout(pongTimeout);
 
@@ -549,7 +567,7 @@ export class WsConnection<
 
             ws.ping();
 
-            this._debugFn("> PING");
+            this._debugFn(`> PING; endpoint: "${this._endpoint}"`);
 
             isWaitingForPong = true;
         }, WS_PING_INTERVAL).unref());

src/ws-connection/utils.ts

@@ -6,5 +6,5 @@ export const exponentiallyWait = ({
 }: { baseDelay?: number; attempt?: number; factor?: number; jitter?: number } = {}): Promise<void> => {
     const delay = Math.round(baseDelay * factor ** attempt + Math.random() * jitter);
 
-    return new Promise(resolve => setTimeout(resolve, delay).unref());
+    return new Promise(resolve => setTimeout(resolve, delay));
 };

test/src/browser/wsdriver/request.ts

@@ -27,6 +27,22 @@ describe("wsdriver/request", () => {
         assert.equal(body, JSON.stringify(options.json));
     });
 
+    it("should decode percent-encoded characters in command", async () => {
+        const url = new URL("http://localhost/session/123/element/abc%3A123%2Fdef/value");
+        const options = { method: "GET" };
+        const connectionOptions = {
+            requestId: 1,
+            sessionPrefix: "/session/123/",
+            compressionType: WsDriverCompression.None,
+        };
+
+        const result = await constructWsDriverRequest(url, options as any, connectionOptions);
+
+        const commandEnd = result.indexOf(0, 8);
+        const command = result.toString("utf8", 8, commandEnd);
+        assert.equal(command, "element/abc:123/def/value");
+    });
+
     it("should construct request with compression if body is large enough", async () => {
         const url = new URL("http://localhost/session/123/element");
         const largeString = "a".repeat(WSD_COMPRESSION_THRESHOLD_BYTES);

test/src/ws-connection/index.ts

@@ -1,3 +1,4 @@
+import * as http from "node:http";
 import { WebSocket, WebSocketServer } from "ws";
 import sinon, { SinonStub, SinonFakeTimers } from "sinon";
 import proxyquire from "proxyquire";
@@ -13,6 +14,7 @@ import {
 import { WS_MAX_REQUEST_ID } from "src/ws-connection/constants";
 
 const STUB_SERVER_PORT = 50123;
+const STUB_HTTP_SERVER_PORT = 50124;
 
 type StubWebSocketServer = WebSocketServer & {
     waitForConnection: Promise<WebSocket>;
@@ -394,4 +396,133 @@ describe('"WsConnection"', () => {
             assert.calledOnce(exponentiallyWaitStub);
         });
     });
+
+    describe("unexpected response handling", () => {
+        const httpEndpoint = `ws://localhost:${STUB_HTTP_SERVER_PORT}`;
+        let httpServer: http.Server | null = null;
+        let httpRequestsCount = 0;
+        let connection: TestWsConnection | null = null;
+
+        const startHttpServer = (
+            responder: (req: http.IncomingMessage, res: http.ServerResponse) => void,
+        ): Promise<void> => {
+            httpRequestsCount = 0;
+            httpServer = http.createServer((req, res) => {
+                httpRequestsCount++;
+                responder(req, res);
+            });
+
+            return new Promise<void>((resolve, reject) => {
+                httpServer!.once("error", reject);
+                httpServer!.listen(STUB_HTTP_SERVER_PORT, () => resolve());
+            });
+        };
+
+        afterEach(async () => {
+            connection?.close();
+            connection = null;
+
+            if (httpServer) {
+                await new Promise<void>(resolve => httpServer!.close(() => resolve()));
+                httpServer = null;
+            }
+        });
+
+        it("should reject with 'WsConnectionEstablishmentError' when server returns non-upgrade response", async () => {
+            await startHttpServer((_req, res) => {
+                res.statusCode = 404;
+                res.end("not a websocket");
+            });
+
+            connection = new TestWsConnectionProxied(httpEndpoint);
+
+            await assert.isRejected(
+                connection.request("Successful.Method"),
+                WsConnectionEstablishmentError,
+                `Couldn't establish WS connection to "${httpEndpoint}"`,
+            );
+        });
+
+        it("should populate error with the response status code", async () => {
+            await startHttpServer((_req, res) => {
+                res.statusCode = 401;
+                res.end("unauthorized");
+            });
+
+            connection = new TestWsConnectionProxied(httpEndpoint);
+
+            try {
+                await connection.request("Successful.Method");
+                assert.fail("should have thrown WsConnectionEstablishmentError");
+            } catch (err) {
+                assert.instanceOf(err, WsConnectionEstablishmentError);
+                assert.equal((err as WsConnectionEstablishmentError).code, 401);
+            }
+        });
+
+        it("should include the response body as the reason in the error message", async () => {
+            await startHttpServer((_req, res) => {
+                res.statusCode = 400;
+                res.end("bad request body from server");
+            });
+
+            connection = new TestWsConnectionProxied(httpEndpoint);
+
+            try {
+                await connection.request("Successful.Method");
+                assert.fail("should have thrown WsConnectionEstablishmentError");
+            } catch (err) {
+                assert.instanceOf(err, WsConnectionEstablishmentError);
+                assert.match((err as Error).message, /Reason: bad request body from server/);
+            }
+        });
+
+        it("should not retry connection on a 4xx unexpected response", async () => {
+            await startHttpServer((_req, res) => {
+                res.statusCode = 403;
+                res.end("forbidden");
+            });
+
+            connection = new TestWsConnectionProxied(httpEndpoint);
+
+            await assert.isRejected(connection.request("Successful.Method"), WsConnectionEstablishmentError);
+
+            assert.equal(httpRequestsCount, 1);
+        });
+
+        it("should retry connection on a non-4xx unexpected response", async () => {
+            await startHttpServer((_req, res) => {
+                res.statusCode = 500;
+                res.end("internal server error");
+            });
+
+            connection = new TestWsConnectionProxied(httpEndpoint);
+
+            await assert.isRejected(connection.request("Successful.Method"), WsConnectionEstablishmentError);
+
+            // 1 initial attempt + 3 retries (retries.count = 3)
+            assert.equal(httpRequestsCount, 4);
+        });
+
+        it("should fall back to 'Unknown reason' when reading the response body fails", async () => {
+            await startHttpServer((_req, res) => {
+                // Promise a 100-byte body, flush the headers (so the WS client sees an
+                // unexpected response), then abruptly close the socket so reading the
+                // body rejects.
+                res.writeHead(403, { "Content-Length": "100" });
+                res.flushHeaders();
+                res.socket?.destroy();
+            });
+
+            connection = new TestWsConnectionProxied(httpEndpoint);
+
+            try {
+                await connection.request("Successful.Method");
+                assert.fail("should have thrown WsConnectionEstablishmentError");
+            } catch (err) {
+                assert.instanceOf(err, WsConnectionEstablishmentError);
+                assert.match((err as Error).message, /Reason: Unknown reason/);
+            }
+        });
+    });
 });