From 2dced017cbb4c2c236ecdc9a568b4ef8bc6b6550 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 6 Apr 2021 10:13:14 -0700 Subject: [PATCH] [SignalR TS] Improve connection error messages (#31402) --- .../clients/ts/FunctionalTests/Startup.cs | 11 +++++++ .../ts/FunctionalTests/ts/ConnectionTests.ts | 14 ++++++++ .../FunctionalTests/ts/HubConnectionTests.ts | 18 ++++++++++ src/SignalR/clients/ts/signalr/src/Errors.ts | 2 +- .../clients/ts/signalr/src/FetchHttpClient.ts | 3 +- .../clients/ts/signalr/src/HttpConnection.ts | 12 +++++-- .../signalr/src/ServerSentEventsTransport.ts | 8 +++-- .../ts/signalr/src/WebSocketTransport.ts | 13 +++++--- .../clients/ts/signalr/src/XhrHttpClient.ts | 2 +- .../ts/signalr/tests/HttpConnection.test.ts | 33 ++++++++++--------- .../tests/HubConnection.Reconnect.test.ts | 2 +- .../tests/ServerSentEventsTransport.test.ts | 4 +-- .../signalr/tests/WebSocketTransport.test.ts | 12 ++++--- 13 files changed, 98 insertions(+), 36 deletions(-) diff --git a/src/SignalR/clients/ts/FunctionalTests/Startup.cs b/src/SignalR/clients/ts/FunctionalTests/Startup.cs index abd16dd939b1..2c2dfd420210 100644 --- 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 index 24dfc61ed729..c9dcbc8fae73 100644 --- 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 index 8dac9a11808f..e8d3d08ef127 100644 --- 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 index 604471dffa05..847e0114e1b5 100644 --- 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 index 52dacce25f9f..a29586c74e6c 100644 --- 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 index dccaf4c57ed6..f93efbc8e29a 100644 --- 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 index e0a6fc46bd58..dbb4a0a6f2b7 100644 --- 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 index 32c26aa5e7ca..d08eb427d71e 100644 --- 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 index fa3e7839d2cc..45805cf78a9a 100644 --- 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 index f25d4ba4ab73..92fcb8f139a6 100644 --- 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 index 756734db2fc5..50ad4a28d8ec 100644 --- 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 index 66a74639c90b..4ad96e7a668b 100644 --- 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 index d3984cd673f7..69b25cd33928 100644 --- 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."); }); }); });