Skip to content

Returning rejected promises in HttpConnection #8315

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 4 commits into from
Mar 11, 2019
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
20 changes: 10 additions & 10 deletions src/SignalR/clients/ts/signalr/src/HttpConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class HttpConnection implements IConnection {

public send(data: string | ArrayBuffer): Promise<void> {
if (this.connectionState !== ConnectionState.Connected) {
throw new Error("Cannot send data if the connection is not in the 'Connected' State.");
return Promise.reject(new Error("Cannot send data if the connection is not in the 'Connected' State."));
}

// Transport will not be null if state is connected
Expand Down Expand Up @@ -157,7 +157,7 @@ export class HttpConnection implements IConnection {
// No fallback or negotiate in this case.
await this.transport!.connect(url, transferFormat);
} else {
throw Error("Negotiation can only be skipped when using the WebSocket transport directly.");
return Promise.reject(new Error("Negotiation can only be skipped when using the WebSocket transport directly."));
}
} else {
let negotiateResponse: INegotiateResponse | null = null;
Expand All @@ -171,11 +171,11 @@ export class HttpConnection implements IConnection {
}

if (negotiateResponse.error) {
throw Error(negotiateResponse.error);
return Promise.reject(new Error(negotiateResponse.error));
}

if ((negotiateResponse as any).ProtocolVersion) {
throw Error("Detected a connection attempt to an ASP.NET SignalR Server. This client only supports connecting to an ASP.NET Core SignalR Server. See https://aka.ms/signalr-core-differences for details.");
return Promise.reject(new Error("Detected a connection attempt to an ASP.NET SignalR Server. This client only supports connecting to an ASP.NET Core SignalR Server. See https://aka.ms/signalr-core-differences for details."));
}

if (negotiateResponse.url) {
Expand All @@ -194,7 +194,7 @@ export class HttpConnection implements IConnection {
while (negotiateResponse.url && redirects < MAX_REDIRECTS);

if (redirects === MAX_REDIRECTS && negotiateResponse.url) {
throw Error("Negotiate redirection limit exceeded.");
return Promise.reject(new Error("Negotiate redirection limit exceeded."));
}

await this.createTransport(url, this.options.transport, negotiateResponse, transferFormat);
Expand All @@ -214,7 +214,7 @@ export class HttpConnection implements IConnection {
this.logger.log(LogLevel.Error, "Failed to start the connection: " + e);
this.connectionState = ConnectionState.Disconnected;
this.transport = undefined;
throw e;
return Promise.reject(e);
}
}

Expand All @@ -238,13 +238,13 @@ export class HttpConnection implements IConnection {
});

if (response.statusCode !== 200) {
throw Error(`Unexpected status code returned from negotiate ${response.statusCode}`);
return Promise.reject(new Error(`Unexpected status code returned from negotiate ${response.statusCode}`));
}

return JSON.parse(response.content as string) as INegotiateResponse;
} catch (e) {
this.logger.log(LogLevel.Error, "Failed to complete negotiation with the server: " + e);
throw e;
return Promise.reject(e);
}
}

Expand Down Expand Up @@ -293,9 +293,9 @@ export class HttpConnection implements IConnection {
}

if (transportExceptions.length > 0) {
throw new Error(`Unable to connect to the server with any of the available transports. ${transportExceptions.join(" ")}`);
return Promise.reject(new Error(`Unable to connect to the server with any of the available transports. ${transportExceptions.join(" ")}`));
}
throw new Error("None of the transports supported by the client are supported by the server.");
return Promise.reject(new Error("None of the transports supported by the client are supported by the server."));
}

private constructTransport(transport: HttpTransportType) {
Expand Down
66 changes: 66 additions & 0 deletions src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,72 @@ describe("HttpConnection", () => {
});
});

it("cannot send with an un-started connection", async () => {
await VerifyLogger.run(async (logger) => {
const connection = new HttpConnection("http://tempuri.org");

await expect(connection.send("LeBron James"))
.rejects
.toThrow("Cannot send data if the connection is not in the 'Connected' State.");
});
});

it("sending before start doesn't throw synchronously", async () => {
await VerifyLogger.run(async (logger) => {
const connection = new HttpConnection("http://tempuri.org");

try {
connection.send("test").catch((e) => {});
} catch (e) {
expect(false).toBe(true);
}

});
});

it("cannot be started if negotiate returns non 200 response", async () => {
await VerifyLogger.run(async (logger) => {
const options: IHttpConnectionOptions = {
...commonOptions,
httpClient: new TestHttpClient()
.on("POST", () => new HttpResponse(999))
.on("GET", () => ""),
logger,
} as IHttpConnectionOptions;

const connection = new HttpConnection("http://tempuri.org", options);
await expect(connection.start(TransferFormat.Text))
.rejects
.toThrow("Unexpected status code returned from negotiate 999");
},
"Failed to start the connection: Error: Unexpected status code returned from negotiate 999");
});

it("all transport failure error get aggregated", async () => {
await VerifyLogger.run(async (loggerImpl) => {
const options: IHttpConnectionOptions = {
WebSocket: false,
...commonOptions,
httpClient: new TestHttpClient()
.on("POST", () => defaultNegotiateResponse)
.on("GET", () => new HttpResponse(200))
.on("DELETE", () => new HttpResponse(202)),

logger: loggerImpl,
transport: HttpTransportType.WebSockets,
} as IHttpConnectionOptions;

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: null ServerSentEvents failed: Error: 'ServerSentEvents' is disabled by the client. LongPolling failed: Error: 'LongPolling' is disabled by the client.");
},
"Failed to start the transport 'WebSockets': null",
"Failed to start the transport 'ServerSentEvents': Error: 'ServerSentEvents' is disabled by the client.",
"Failed to start the transport 'LongPolling': Error: 'LongPolling' is disabled by the client.",
"Failed to start the connection: Error: Unable to connect to the server with any of the available transports. WebSockets failed: null ServerSentEvents failed: Error: 'ServerSentEvents' is disabled by the client. LongPolling failed: Error: 'LongPolling' is disabled by the client.");
});

it("can stop a non-started connection", async () => {
await VerifyLogger.run(async (logger) => {
const connection = new HttpConnection("http://tempuri.org", { ...commonOptions, logger });
Expand Down