Cleanup FetchHttpClient

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);
     });
 }

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

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();
 

src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts

@@ -14,113 +14,104 @@ export class FetchHttpClient extends HttpClient {
     }
 
     /** @inheritDoc */
-    public send(request: HttpRequest): Promise<HttpResponse> {
+    public async send(request: HttpRequest): Promise<HttpResponse> {
         // 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<HttpResponse>((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<any> {
+function deserializeContent(response: Response, responseType?: XMLHttpRequestResponseType): Promise<string | ArrayBuffer> {
     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;

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,

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";