From 7317bb16a9cd5a834c0468a74113055031262217 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Wed, 28 Aug 2019 11:13:53 -0700 Subject: [PATCH] Cleanup FetchHttpClient --- .../clients/ts/FunctionalTests/ts/Common.ts | 33 ++++- .../ts/FunctionalTests/ts/ConnectionTests.ts | 2 +- .../FunctionalTests/ts/HubConnectionTests.ts | 42 +++--- .../clients/ts/signalr/src/FetchHttpClient.ts | 131 ++++++++---------- .../clients/ts/signalr/src/HttpClient.ts | 8 ++ src/SignalR/clients/ts/signalr/src/index.ts | 2 - 6 files changed, 120 insertions(+), 98 deletions(-) diff --git a/src/SignalR/clients/ts/FunctionalTests/ts/Common.ts b/src/SignalR/clients/ts/FunctionalTests/ts/Common.ts index 15c03ef6d0..c66bff8e49 100644 --- a/src/SignalR/clients/ts/FunctionalTests/ts/Common.ts +++ b/src/SignalR/clients/ts/FunctionalTests/ts/Common.ts @@ -1,10 +1,15 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -import { DefaultHttpClient, FetchHttpClient, HttpClient, HttpTransportType, IHubProtocol, JsonHubProtocol, XhrHttpClient } from "@microsoft/signalr"; +import { HttpClient, HttpTransportType, IHubProtocol, JsonHubProtocol } from "@microsoft/signalr"; import { MessagePackHubProtocol } from "@microsoft/signalr-protocol-msgpack"; import { TestLogger } from "./TestLogger"; +import { FetchHttpClient } from "@microsoft/signalr/dist/esm/FetchHttpClient"; +import { NodeHttpClient } from "@microsoft/signalr/dist/esm/NodeHttpClient"; +import { Platform } from "@microsoft/signalr/dist/esm/Utils"; +import { XhrHttpClient } from "@microsoft/signalr/dist/esm/XhrHttpClient"; + // On slower CI machines, these tests sometimes take longer than 5s jasmine.DEFAULT_TIMEOUT_INTERVAL = 20 * 1000; @@ -98,14 +103,34 @@ export function eachTransportAndProtocol(action: (transport: HttpTransportType, }); } +export function eachTransportAndProtocolAndHttpClient(action: (transport: HttpTransportType, protocol: IHubProtocol, httpClient: HttpClient) => void) { + eachTransportAndProtocol((transport, protocol) => { + getHttpClients().forEach((httpClient) => { + action(transport, protocol, httpClient); + }); + }); +} + export function getGlobalObject(): any { return typeof window !== "undefined" ? window : global; } -export function eachHttpClient(action: (transport: HttpClient) => void) { - const httpClients: HttpClient[] = [new FetchHttpClient(TestLogger.instance), new XhrHttpClient(TestLogger.instance), new DefaultHttpClient(TestLogger.instance)]; +export function getHttpClients(): HttpClient[] { + const httpClients: HttpClient[] = []; + if (typeof XMLHttpRequest !== "undefined") { + httpClients.push(new XhrHttpClient(TestLogger.instance)); + } + if (typeof fetch !== "undefined") { + httpClients.push(new FetchHttpClient(TestLogger.instance)); + } + if (Platform.isNode) { + httpClients.push(new NodeHttpClient(TestLogger.instance)); + } + return httpClients; +} - return httpClients.forEach((t) => { +export function eachHttpClient(action: (transport: HttpClient) => void) { + return getHttpClients().forEach((t) => { return action(t); }); } diff --git a/src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts b/src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts index 9d60a38ff6..3b559af265 100644 --- a/src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts +++ b/src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts @@ -45,7 +45,7 @@ describe("connection", () => { eachTransport((transportType) => { eachHttpClient((httpClient) => { - describe(`over ${HttpTransportType[transportType]}`, () => { + describe(`over ${HttpTransportType[transportType]} with ${(httpClient.constructor as any).name}`, () => { it("can send and receive messages", (done) => { const message = "Hello World!"; // the url should be resolved relative to the document.location.host diff --git a/src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts b/src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts index 3d5f434a17..dbfa1a886f 100644 --- a/src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts +++ b/src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts @@ -7,7 +7,7 @@ import { AbortError, DefaultHttpClient, HttpClient, HttpRequest, HttpResponse, HttpTransportType, HubConnectionBuilder, IHttpConnectionOptions, JsonHubProtocol, NullLogger } from "@microsoft/signalr"; import { MessagePackHubProtocol } from "@microsoft/signalr-protocol-msgpack"; -import { eachTransport, eachTransportAndProtocol, ENDPOINT_BASE_HTTPS_URL, ENDPOINT_BASE_URL } from "./Common"; +import { eachTransport, eachTransportAndProtocolAndHttpClient, ENDPOINT_BASE_HTTPS_URL, ENDPOINT_BASE_URL } from "./Common"; import "./LogBannerReporter"; import { TestLogger } from "./TestLogger"; @@ -49,12 +49,12 @@ function getConnectionBuilder(transportType?: HttpTransportType, url?: string, o } describe("hubConnection", () => { - eachTransportAndProtocol((transportType, protocol) => { + eachTransportAndProtocolAndHttpClient((transportType, protocol, httpClient) => { describe("using " + protocol.name + " over " + HttpTransportType[transportType] + " transport", () => { it("can invoke server method and receive result", (done) => { const message = "你好，世界！"; - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -81,7 +81,7 @@ describe("hubConnection", () => { it("using https, can invoke server method and receive result", (done) => { const message = "你好，世界！"; - const hubConnection = getConnectionBuilder(transportType, TESTHUBENDPOINT_HTTPS_URL) + const hubConnection = getConnectionBuilder(transportType, TESTHUBENDPOINT_HTTPS_URL, { httpClient }) .withHubProtocol(protocol) .build(); @@ -108,7 +108,7 @@ describe("hubConnection", () => { it("can invoke server method non-blocking and not receive result", (done) => { const message = "你好，世界！"; - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -130,7 +130,7 @@ describe("hubConnection", () => { }); it("can invoke server method structural object and receive structural result", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -154,7 +154,7 @@ describe("hubConnection", () => { }); it("can stream server method and receive result", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -185,7 +185,7 @@ describe("hubConnection", () => { }); it("can stream server method and cancel stream", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -219,7 +219,7 @@ describe("hubConnection", () => { it("rethrows an exception from the server when invoking", (done) => { const errorMessage = "An unexpected error occurred invoking 'ThrowException' on the server. InvalidOperationException: An error occurred."; - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -241,7 +241,7 @@ describe("hubConnection", () => { }); it("throws an exception when invoking streaming method with invoke", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -263,7 +263,7 @@ describe("hubConnection", () => { }); it("throws an exception when receiving a streaming result for method called with invoke", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -286,7 +286,7 @@ describe("hubConnection", () => { it("rethrows an exception from the server when streaming", (done) => { const errorMessage = "An unexpected error occurred invoking 'StreamThrowException' on the server. InvalidOperationException: An error occurred."; - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -313,7 +313,7 @@ describe("hubConnection", () => { }); it("throws an exception when invoking hub method with stream", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -340,7 +340,7 @@ describe("hubConnection", () => { }); it("can receive server calls", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -370,7 +370,7 @@ describe("hubConnection", () => { }); it("can receive server calls without rebinding handler when restarted", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -425,7 +425,7 @@ describe("hubConnection", () => { }); it("closed with error or start fails if hub cannot be created", async (done) => { - const hubConnection = getConnectionBuilder(transportType, ENDPOINT_BASE_URL + "/uncreatable") + const hubConnection = getConnectionBuilder(transportType, ENDPOINT_BASE_URL + "/uncreatable", { httpClient }) .withHubProtocol(protocol) .build(); @@ -446,7 +446,7 @@ describe("hubConnection", () => { }); it("can handle different types", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -489,7 +489,7 @@ describe("hubConnection", () => { }); it("can receive different types", (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -534,7 +534,7 @@ describe("hubConnection", () => { it("can be restarted", (done) => { const message = "你好，世界！"; - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -577,7 +577,7 @@ describe("hubConnection", () => { }); it("can stream from client to server with rxjs", async (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); @@ -594,7 +594,7 @@ describe("hubConnection", () => { }); it("can stream from client to server and close with error with rxjs", async (done) => { - const hubConnection = getConnectionBuilder(transportType) + const hubConnection = getConnectionBuilder(transportType, undefined, { httpClient }) .withHubProtocol(protocol) .build(); diff --git a/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts b/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts index 0b4c2754a7..48840b42c2 100644 --- a/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts +++ b/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts @@ -14,113 +14,104 @@ export class FetchHttpClient extends HttpClient { } /** @inheritDoc */ - public send(request: HttpRequest): Promise { + public async send(request: HttpRequest): Promise { // Check that abort was not signaled before calling send if (request.abortSignal && request.abortSignal.aborted) { - return Promise.reject(new AbortError()); + throw new AbortError(); } if (!request.method) { - return Promise.reject(new Error("No method defined.")); + throw new Error("No method defined."); } if (!request.url) { - return Promise.reject(new Error("No url defined.")); + throw new Error("No url defined."); } - return new Promise((resolve, reject) => { - const abortController = new AbortController(); + const abortController = new AbortController(); - const fetchRequest = new Request(request.url!, { + let error: any; + // Hook our abortSignal into the abort controller + if (request.abortSignal) { + request.abortSignal.onabort = () => { + abortController.abort(); + error = new AbortError(); + }; + } + + // If a timeout has been passed in, setup a timeout to call abort + // Type needs to be any to fit window.setTimeout and NodeJS.setTimeout + let timeoutId: any = null; + if (request.timeout) { + const msTimeout = request.timeout!; + timeoutId = setTimeout(() => { + abortController.abort(); + this.logger.log(LogLevel.Warning, `Timeout from HTTP request.`); + error = new TimeoutError(); + }, msTimeout); + } + + let response: Response; + try { + response = await fetch(request.url!, { body: request.content!, cache: "no-cache", credentials: "include", headers: { "Content-Type": "text/plain;charset=UTF-8", - "X-Requested-With": "Fetch", + "X-Requested-With": "XMLHttpRequest", ...request.headers, }, method: request.method!, mode: "cors", + redirect: "manual", signal: abortController.signal, }); - - // Hook our abourtSignal into the abort controller + } catch (e) { + if (error) { + throw error; + } + this.logger.log( + LogLevel.Warning, + `Error from HTTP request. ${e}.`, + ); + throw e; + } finally { + if (timeoutId) { + clearTimeout(timeoutId); + } if (request.abortSignal) { - request.abortSignal.onabort = () => { - abortController.abort(); - reject(new AbortError()); - }; + request.abortSignal.onabort = null; } + } - // If a timeout has been passed in setup a timeout to call abort - // Type needs to be any to fit window.setTimeout and NodeJS.setTimeout - let timeoutId: any = null; - if (request.timeout) { - const msTimeout = request.timeout!; - timeoutId = setTimeout(() => { - abortController.abort(); - reject(new TimeoutError()); - }, msTimeout); - } + if (!response.ok) { + throw new HttpError(response.statusText, response.status); + } - fetch(fetchRequest) - .then((response: Response) => { - if (timeoutId) { - clearTimeout(timeoutId); - } - if (!response.ok) { - throw new Error(`${response.status}: ${response.statusText}.`); - } else { - return response; - } - }) - .then((response: Response) => { - if (request.abortSignal) { - request.abortSignal.onabort = null; - } + const content = deserializeContent(response, request.responseType); + const payload = await content; - const content = deserializeContent(response, request.responseType); - - content.then((payload) => { - resolve(new HttpResponse( - response.status, - response.statusText, - payload, - )); - }).catch(() => { - reject(new HttpError(response.statusText, response.status)); - }); - }) - .catch((error) => { - this.logger.log( - LogLevel.Warning, - `Error from HTTP request. ${error.message}.`, - ); - const [statusText, status] = error.message.split(":"); - reject(new HttpError(statusText, status)); - }); - }); + return new HttpResponse( + response.status, + response.statusText, + payload, + ); } } -function deserializeContent(response: Response, responseType?: XMLHttpRequestResponseType): Promise { +function deserializeContent(response: Response, responseType?: XMLHttpRequestResponseType): Promise { let content; switch (responseType) { case "arraybuffer": content = response.arrayBuffer(); break; - case "blob": - content = response.blob(); - break; - case "document": - content = response.json(); - break; - case "json": - content = response.json(); - break; case "text": content = response.text(); break; + case "blob": + case "document": + case "json": + throw new Error(`${responseType} is not supported.`); default: content = response.text(); break; diff --git a/src/SignalR/clients/ts/signalr/src/HttpClient.ts b/src/SignalR/clients/ts/signalr/src/HttpClient.ts index 9685feca5e..c50289bbf1 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpClient.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpClient.ts @@ -57,6 +57,14 @@ export class HttpResponse { * @param {ArrayBuffer} content The content of the response. */ constructor(statusCode: number, statusText: string, content: ArrayBuffer); + + /** Constructs a new instance of {@link @microsoft/signalr.HttpResponse} with the specified status code, message and binary content. + * + * @param {number} statusCode The status code of the response. + * @param {string} statusText The status message of the response. + * @param {string | ArrayBuffer} content The content of the response. + */ + constructor(statusCode: number, statusText: string, content: string | ArrayBuffer); constructor( public readonly statusCode: number, public readonly statusText?: string, diff --git a/src/SignalR/clients/ts/signalr/src/index.ts b/src/SignalR/clients/ts/signalr/src/index.ts index 8d93b7530f..dc2086c661 100644 --- a/src/SignalR/clients/ts/signalr/src/index.ts +++ b/src/SignalR/clients/ts/signalr/src/index.ts @@ -10,7 +10,6 @@ export { AbortSignal } from "./AbortController"; export { AbortError, HttpError, TimeoutError } from "./Errors"; export { HttpClient, HttpRequest, HttpResponse } from "./HttpClient"; export { DefaultHttpClient } from "./DefaultHttpClient"; -export { FetchHttpClient } from "./FetchHttpClient"; export { IHttpConnectionOptions } from "./IHttpConnectionOptions"; export { HubConnection, HubConnectionState } from "./HubConnection"; export { HubConnectionBuilder } from "./HubConnectionBuilder"; @@ -23,4 +22,3 @@ export { NullLogger } from "./Loggers"; export { JsonHubProtocol } from "./JsonHubProtocol"; export { Subject } from "./Subject"; export { IRetryPolicy, RetryContext } from "./IRetryPolicy"; -export { XhrHttpClient } from "./XhrHttpClient";