Skip to content

[SignalR TS] Improve connection error messages #31402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/SignalR/clients/ts/FunctionalTests/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'"));
}
})
});
});
});
18 changes: 18 additions & 0 deletions src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/clients/ts/signalr/src/Errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions src/SignalR/clients/ts/signalr/src/HttpConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏🏾 This error message is EF level good.

cc @ajcvickers

+ " If you have multiple servers check that sticky sessions are enabled."));
}
};

Expand Down
13 changes: 8 additions & 5 deletions src/SignalR/clients/ts/signalr/src/WebSocketTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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));
}
};
});
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/clients/ts/signalr/src/XhrHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
};

Expand Down
33 changes: 18 additions & 15 deletions src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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'");
});
});

Expand Down Expand Up @@ -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;
Expand All @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down Expand Up @@ -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();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand All @@ -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);
});
Expand Down Expand Up @@ -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.");
});
});
});
Expand Down