[SignalR TS] Improve connection error messages (#31402)

dotnet · Apr 6, 2021 · 2dced01 · 2dced01
1 parent ab2704a
commit 2dced01
Show file tree

Hide file tree

Showing 13 changed files with 98 additions and 36 deletions.
diff --git a/src/SignalR/clients/ts/FunctionalTests/Startup.cs b/src/SignalR/clients/ts/FunctionalTests/Startup.cs
@@ -197,6 +197,17 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env, ILogger<
                 await next.Invoke();
             });
 
+            app.Use((context, next) =>
+            {
+                if (context.Request.Path.StartsWithSegments("/bad-negotiate"))
+                {
+                    context.Response.StatusCode = 400;
+                    return context.Response.WriteAsync("Some response from server");
+                }
+
+                return next();
+            });
+
             app.UseRouting();
 
             // Custom CORS to allow any origin + credentials (which isn't allowed by the CORS spec)

diff --git a/src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts b/src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts
@@ -222,6 +222,20 @@ describe("connection", () => {
 
                 await closePromise;
             });
+
+            it("contains server response in error", async () => {
+                const connection = new HttpConnection(ENDPOINT_BASE_URL + "/bad-negotiate", {
+                    ...commonOptions,
+                    httpClient,
+                });
+
+                try {
+                    await connection.start(TransferFormat.Text);
+                    expect(true).toBe(false);
+                } catch (e) {
+                    expect(e).toEqual(new Error("Failed to complete negotiation with the server: Error: Some response from server: Status code '400'"));
+                }
+            })
         });
     });
 });
diff --git a/src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts b/src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts
@@ -558,6 +558,24 @@ describe("hubConnection", () => {
                 }
             });
 
+            it("can get error from unauthorized hub connection", async (done) => {
+                try {
+                    const hubConnection = getConnectionBuilder(transportType, ENDPOINT_BASE_URL + "/authorizedhub").build();
+
+                    hubConnection.onclose((error) => {
+                        expect(error).toBe(undefined);
+                        done();
+                    });
+
+                    await hubConnection.start();
+
+                    fail("shouldn't reach here");
+                } catch (err) {
+                    expect(err).toEqual(new Error("Failed to complete negotiation with the server: Error: Unauthorized: Status code '401'"));
+                    done();
+                }
+            });
+
             if (transportType !== HttpTransportType.LongPolling) {
                 it("terminates if no messages received within timeout interval", async (done) => {
                     const hubConnection = getConnectionBuilder(transportType).build();

diff --git a/src/SignalR/clients/ts/signalr/src/Errors.ts b/src/SignalR/clients/ts/signalr/src/Errors.ts
@@ -17,7 +17,7 @@ export class HttpError extends Error {
      */
     constructor(errorMessage: string, statusCode: number) {
         const trueProto = new.target.prototype;
-        super(errorMessage);
+        super(`${errorMessage}: Status code '${statusCode}'`);
         this.statusCode = statusCode;
 
         // Workaround issue in Typescript compiler

diff --git a/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts b/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts
@@ -113,7 +113,8 @@ export class FetchHttpClient extends HttpClient {
         }
 
         if (!response.ok) {
-            throw new HttpError(response.statusText, response.status);
+            const errorMessage = await deserializeContent(response, "text") as string;
+            throw new HttpError(errorMessage || response.statusText, response.status);
         }
 
         const content = deserializeContent(response, request.responseType);

diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 import { DefaultHttpClient } from "./DefaultHttpClient";
+import { HttpError } from "./Errors";
 import { HttpClient } from "./HttpClient";
 import { IConnection } from "./IConnection";
 import { IHttpConnectionOptions } from "./IHttpConnectionOptions";
@@ -334,8 +335,15 @@ export class HttpConnection implements IConnection {
             }
             return negotiateResponse;
         } catch (e) {
-            this._logger.log(LogLevel.Error, "Failed to complete negotiation with the server: " + e);
-            return Promise.reject(e);
+            let errorMessage = "Failed to complete negotiation with the server: " + e;
+            if (e instanceof HttpError) {
+                if (e.statusCode === 404) {
+                    errorMessage = errorMessage + " Either this is not a SignalR endpoint or there is a proxy blocking the connection.";
+                }
+            }
+            this._logger.log(LogLevel.Error, errorMessage);
+
+            return Promise.reject(new Error(errorMessage));
         }
     }
 

diff --git a/src/SignalR/clients/ts/signalr/src/ServerSentEventsTransport.ts b/src/SignalR/clients/ts/signalr/src/ServerSentEventsTransport.ts
@@ -90,11 +90,13 @@ export class ServerSentEventsTransport implements ITransport {
 
                 // @ts-ignore: not using event on purpose
                 eventSource.onerror = (e: Event) => {
-                    const error = new Error("Error occurred while starting EventSource");
+                    // EventSource doesn't give any useful information about server side closes.
                     if (opened) {
-                        this._close(error);
+                        this._close();
                     } else {
-                        reject(error);
+                        reject(new Error("EventSource failed to connect. The connection could not be found on the server,"
+                        + " either the connection ID is not present on the server, or a proxy is refusing/buffering the connection."
+                        + " If you have multiple servers check that sticky sessions are enabled."));
                     }
                 };
 

diff --git a/src/SignalR/clients/ts/signalr/src/WebSocketTransport.ts b/src/SignalR/clients/ts/signalr/src/WebSocketTransport.ts
@@ -91,10 +91,10 @@ export class WebSocketTransport implements ITransport {
                 if (typeof ErrorEvent !== "undefined" && event instanceof ErrorEvent) {
                     error = event.error;
                 } else {
-                    error = new Error("There was an error with the transport.");
+                    error = "There was an error with the transport";
                 }
 
-                reject(error);
+                this._logger.log(LogLevel.Information, `(WebSockets transport) ${error}.`);
             };
 
             webSocket.onmessage = (message: MessageEvent) => {
@@ -120,10 +120,13 @@ export class WebSocketTransport implements ITransport {
                     if (typeof ErrorEvent !== "undefined" && event instanceof ErrorEvent) {
                         error = event.error;
                     } else {
-                        error = new Error("There was an error with the transport.");
+                        error = "WebSocket failed to connect. The connection could not be found on the server,"
+                        + " either the endpoint may not be a SignalR endpoint,"
+                        + " the connection ID is not present on the server, or there is a proxy blocking WebSockets."
+                        + " If you have multiple servers check that sticky sessions are enabled.";
                     }
 
-                    reject(error);
+                    reject(new Error(error));
                 }
             };
         });
@@ -163,7 +166,7 @@ export class WebSocketTransport implements ITransport {
         this._logger.log(LogLevel.Trace, "(WebSockets transport) socket closed.");
         if (this.onclose) {
             if (this._isCloseEvent(event) && (event.wasClean === false || event.code !== 1000)) {
-                this.onclose(new Error(`WebSocket closed with status code: ${event.code} (${event.reason}).`));
+                this.onclose(new Error(`WebSocket closed with status code: ${event.code} (${event.reason || "no reason given"}).`));
             } else if (event instanceof Error) {
                 this.onclose(event);
             } else {

diff --git a/src/SignalR/clients/ts/signalr/src/XhrHttpClient.ts b/src/SignalR/clients/ts/signalr/src/XhrHttpClient.ts
@@ -67,7 +67,7 @@ export class XhrHttpClient extends HttpClient {
                 if (xhr.status >= 200 && xhr.status < 300) {
                     resolve(new HttpResponse(xhr.status, xhr.statusText, xhr.response || xhr.responseText));
                 } else {
-                    reject(new HttpError(xhr.statusText, xhr.status));
+                    reject(new HttpError(xhr.response || xhr.responseText || xhr.statusText, xhr.status));
                 }
             };
 

diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts
@@ -67,9 +67,9 @@ describe("HttpConnection", () => {
 
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toBe("error");
+                .toThrow(new Error("Failed to complete negotiation with the server: error"));
         },
-        "Failed to start the connection: error",
+        "Failed to start the connection: Error: Failed to complete negotiation with the server: error",
         "Failed to complete negotiation with the server: error");
     });
 
@@ -125,14 +125,14 @@ describe("HttpConnection", () => {
 
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toBe("reached negotiate.");
+                .toThrow(new Error("Failed to complete negotiation with the server: reached negotiate."));
 
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toBe("reached negotiate.");
+                .toThrow(new Error("Failed to complete negotiation with the server: reached negotiate."));
         },
         "Failed to complete negotiation with the server: reached negotiate.",
-        "Failed to start the connection: reached negotiate.");
+        "Failed to start the connection: Error: Failed to complete negotiation with the server: reached negotiate.");
     });
 
     it("can stop a starting connection", async () => {
@@ -229,14 +229,16 @@ describe("HttpConnection", () => {
             const connection = new HttpConnection("http://tempuri.org", options);
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toThrow("Unable to connect to the server with any of the available transports. WebSockets failed: Error: There was an error with the transport. " +
-                "ServerSentEvents failed: Error: 'ServerSentEvents' is disabled by the client. LongPolling failed: Error: 'LongPolling' is disabled by the client.");
+                .toThrow("Unable to connect to the server with any of the available transports. WebSockets failed: Error: WebSocket failed to connect. " +
+                "The connection could not be found on the server, either the endpoint may not be a SignalR endpoint, the connection ID is not present on the server, " +
+                "or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled. ServerSentEvents failed: Error: 'ServerSentEvents' is disabled by the client. LongPolling failed: Error: 'LongPolling' is disabled by the client.");
 
             expect(negotiateCount).toEqual(1);
         },
-        "Failed to start the transport 'WebSockets': Error: There was an error with the transport.",
-        "Failed to start the connection: Error: Unable to connect to the server with any of the available transports. WebSockets failed: Error: There was an error with the transport. " +
-        "ServerSentEvents failed: Error: 'ServerSentEvents' is disabled by the client. LongPolling failed: Error: 'LongPolling' is disabled by the client.");
+        "Failed to start the transport 'WebSockets': Error: WebSocket failed to connect. The connection could not be found on the server, " +
+        "either the endpoint may not be a SignalR endpoint, the connection ID is not present on the server, or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled.",
+        "Failed to start the connection: Error: Unable to connect to the server with any of the available transports. WebSockets failed: " +
+        "Error: WebSocket failed to connect. The connection could not be found on the server, either the endpoint may not be a SignalR endpoint, the connection ID is not present on the server, or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled. ServerSentEvents failed: Error: 'ServerSentEvents' is disabled by the client. LongPolling failed: Error: 'LongPolling' is disabled by the client.");
     });
 
     it("negotiate called again when transport fails to start and falls back", async () => {
@@ -300,7 +302,7 @@ describe("HttpConnection", () => {
         },
         "Failed to start the transport 'WebSockets': Error: Don't allow Websockets.",
         "Failed to complete negotiation with the server: Error: negotiate failed",
-        "Failed to start the connection: Error: negotiate failed");
+        "Failed to start the connection: Error: Failed to complete negotiation with the server: Error: negotiate failed");
     });
 
     it("can stop a non-started connection", async () => {
@@ -399,8 +401,8 @@ describe("HttpConnection", () => {
                     await connection.stop();
                 }
             },
-            "Failed to complete negotiation with the server: Error: We don't care how this turns out",
-            "Failed to start the connection: Error: We don't care how this turns out");
+            "Failed to complete negotiation with the server: Error: We don't care how this turns out: Status code '500'",
+            "Failed to start the connection: Error: Failed to complete negotiation with the server: Error: We don't care how this turns out: Status code '500'");
         });
     });
 
@@ -1122,7 +1124,7 @@ describe("HttpConnection", () => {
 
             await TestWebSocket.webSocketSet;
             await TestWebSocket.webSocket.closeSet;
-            TestWebSocket.webSocket.onerror(new TestEvent());
+            TestWebSocket.webSocket.onclose(new TestEvent());
 
             try {
                 await startPromise;
@@ -1133,7 +1135,8 @@ describe("HttpConnection", () => {
 
             await connection.stop();
         },
-        "Failed to start the transport 'WebSockets': Error: There was an error with the transport.");
+        "Failed to start the transport 'WebSockets': Error: WebSocket failed to connect. The connection could not be found on the server, " +
+        "either the endpoint may not be a SignalR endpoint, the connection ID is not present on the server, or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled.");
     });
 
     it("user agent header set on negotiate", async () => {

diff --git a/src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts b/src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts
@@ -876,6 +876,6 @@ describe("auto reconnect", () => {
             expect(closeCount).toBe(1);
         },
         "Failed to complete negotiation with the server: Error with negotiate",
-        "Failed to start the connection: Error with negotiate");
+        "Failed to start the connection: Error: Failed to complete negotiation with the server: Error with negotiate");
     });
 });
diff --git a/src/SignalR/clients/ts/signalr/tests/ServerSentEventsTransport.test.ts b/src/SignalR/clients/ts/signalr/tests/ServerSentEventsTransport.test.ts
@@ -63,7 +63,7 @@ describe("ServerSentEventsTransport", () => {
 
             await expect(connectPromise)
                 .rejects
-                .toEqual(new Error("Error occurred while starting EventSource"));
+                .toEqual(new Error("EventSource failed to connect. The connection could not be found on the server, either the connection ID is not present on the server, or a proxy is refusing/buffering the connection. If you have multiple servers check that sticky sessions are enabled."));
             expect(closeCalled).toBe(false);
         });
     });
@@ -164,7 +164,7 @@ describe("ServerSentEventsTransport", () => {
 
             expect(closeCalled).toBe(true);
             expect(TestEventSource.eventSource.closed).toBe(true);
-            expect(error).toEqual(new Error("Error occurred while starting EventSource"));
+            expect(error).toBeUndefined();
         });
     });
 

diff --git a/src/SignalR/clients/ts/signalr/tests/WebSocketTransport.test.ts b/src/SignalR/clients/ts/signalr/tests/WebSocketTransport.test.ts
@@ -59,11 +59,12 @@ describe("WebSocketTransport", () => {
 
             expect(connectComplete).toBe(false);
 
-            TestWebSocket.webSocket.onerror(new TestEvent());
+            TestWebSocket.webSocket.onclose(new TestEvent());
 
             await expect(connectPromise)
                 .rejects
-                .toThrow("There was an error with the transport.");
+                .toThrow("WebSocket failed to connect. The connection could not be found on the server, either the endpoint may not be a SignalR endpoint, " +
+                "the connection ID is not present on the server, or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled.");
             expect(connectComplete).toBe(false);
         });
     });
@@ -89,7 +90,8 @@ describe("WebSocketTransport", () => {
 
             await expect(connectPromise)
                 .rejects
-                .toThrow("There was an error with the transport.");
+                .toThrow("WebSocket failed to connect. The connection could not be found on the server, either the endpoint may not be a SignalR endpoint, " +
+                "the connection ID is not present on the server, or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled.");
             expect(connectComplete).toBe(false);
             expect(closeCalled).toBe(false);
         });
@@ -340,8 +342,8 @@ describe("WebSocketTransport", () => {
             expect(closeCalled).toBe(false);
             expect(error!).toBeUndefined();
 
-            TestWebSocket.webSocket.onerror(new TestEvent());
-            await expect(connectPromise).rejects.toThrow("There was an error with the transport.");
+            await expect(connectPromise).rejects.toThrow("WebSocket failed to connect. The connection could not be found on the server, " +
+            "either the endpoint may not be a SignalR endpoint, the connection ID is not present on the server, or there is a proxy blocking WebSockets. If you have multiple servers check that sticky sessions are enabled.");
         });
     });
 });