Fix #8974 by supporting strings in configureLogging for SignalR JS client (#9252)

src/SignalR/clients/ts/signalr/src/HubConnectionBuilder.ts

@@ -13,6 +13,31 @@ import { JsonHubProtocol } from "./JsonHubProtocol";
 import { NullLogger } from "./Loggers";
 import { Arg, ConsoleLogger } from "./Utils";
 
+// tslint:disable:object-literal-sort-keys
+const LogLevelNameMapping = {
+    trace: LogLevel.Trace,
+    debug: LogLevel.Debug,
+    info: LogLevel.Information,
+    information: LogLevel.Information,
+    warn: LogLevel.Warning,
+    warning: LogLevel.Warning,
+    error: LogLevel.Error,
+    critical: LogLevel.Critical,
+    none: LogLevel.None,
+};
+
+function parseLogLevel(name: string): LogLevel {
+    // Case-insensitive matching via lower-casing
+    // Yes, I know case-folding is a complicated problem in Unicode, but we only support
+    // the ASCII strings defined in LogLevelNameMapping anyway, so it's fine -anurse.
+    const mapping = LogLevelNameMapping[name.toLowerCase()];
+    if (typeof mapping !== "undefined") {
+        return mapping;
+    } else {
+        throw new Error(`Unknown log level: ${name}`);
+    }
+}
+
 /** A builder for configuring {@link @aspnet/signalr.HubConnection} instances. */
 export class HubConnectionBuilder {
     /** @internal */
@@ -44,15 +69,26 @@ export class HubConnectionBuilder {
 
     /** Configures custom logging for the {@link @aspnet/signalr.HubConnection}.
      *
-     * @param {LogLevel | ILogger} logging An object implementing the {@link @aspnet/signalr.ILogger} interface or {@link @aspnet/signalr.LogLevel}.
+     * @param {string} logLevel A string representing a LogLevel setting a minimum level of messages to log.
+     *    See {@link https://docs.microsoft.com/en-us/aspnet/core/signalr/configuration#configure-logging|the documentation for client logging configuration} for more details.
+     */
+    public configureLogging(logLevel: string): HubConnectionBuilder;
+
+    /** Configures custom logging for the {@link @aspnet/signalr.HubConnection}.
+     *
+     * @param {LogLevel | string | ILogger} logging A {@link @aspnet/signalr.LogLevel}, a string representing a LogLevel, or an object implementing the {@link @aspnet/signalr.ILogger} interface.
+     *    See {@link https://docs.microsoft.com/en-us/aspnet/core/signalr/configuration#configure-logging|the documentation for client logging configuration} for more details.
      * @returns The {@link @aspnet/signalr.HubConnectionBuilder} instance, for chaining.
      */
-    public configureLogging(logging: LogLevel | ILogger): HubConnectionBuilder;
+    public configureLogging(logging: LogLevel | string | ILogger): HubConnectionBuilder;
-    public configureLogging(logging: LogLevel | ILogger): HubConnectionBuilder {
+    public configureLogging(logging: LogLevel | string | ILogger): HubConnectionBuilder {
         Arg.isRequired(logging, "logging");
 
         if (isLogger(logging)) {
             this.logger = logging;
+        } else if (typeof logging === "string") {
+            const logLevel = parseLogLevel(logging);
+            this.logger = new ConsoleLogger(logLevel);
         } else {
             this.logger = new ConsoleLogger(logging);
         }
@@ -92,7 +128,7 @@ export class HubConnectionBuilder {
         // Flow-typing knows where it's at. Since HttpTransportType is a number and IHttpConnectionOptions is guaranteed
         // to be an object, we know (as does TypeScript) this comparison is all we need to figure out which overload was called.
         if (typeof transportTypeOrOptions === "object") {
-            this.httpConnectionOptions = {...this.httpConnectionOptions, ...transportTypeOrOptions};
+            this.httpConnectionOptions = { ...this.httpConnectionOptions, ...transportTypeOrOptions };
         } else {
             this.httpConnectionOptions = {
                 ...this.httpConnectionOptions,

src/SignalR/clients/ts/signalr/src/Utils.ts

@@ -147,8 +147,17 @@ export class SubjectSubscription<T> implements ISubscription<T> {
 export class ConsoleLogger implements ILogger {
     private readonly minimumLogLevel: LogLevel;
 
+    // Public for testing purposes.
+    public outputConsole: {
+        error(message: any): void,
+        warn(message: any): void,
+        info(message: any): void,
+        log(message: any): void,
+    };
+
     constructor(minimumLogLevel: LogLevel) {
         this.minimumLogLevel = minimumLogLevel;
+        this.outputConsole = console;
     }
 
     public log(logLevel: LogLevel, message: string): void {
@@ -156,17 +165,17 @@ export class ConsoleLogger implements ILogger {
             switch (logLevel) {
                 case LogLevel.Critical:
                 case LogLevel.Error:
-                    console.error(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
+                    this.outputConsole.error(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
                     break;
                 case LogLevel.Warning:
-                    console.warn(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
+                    this.outputConsole.warn(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
                     break;
                 case LogLevel.Information:
-                    console.info(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
+                    this.outputConsole.info(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
                     break;
                 default:
                     // console.debug only goes to attached debuggers in Node, so we use console.log for Trace and Debug
-                    console.log(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
+                    this.outputConsole.log(`[${new Date().toISOString()}] ${LogLevel[logLevel]}: ${message}`);
                     break;
             }
         }

src/SignalR/clients/ts/signalr/tests/HubConnectionBuilder.test.ts

@@ -10,6 +10,7 @@ import { HubMessage, IHubProtocol } from "../src/IHubProtocol";
 import { ILogger, LogLevel } from "../src/ILogger";
 import { HttpTransportType, TransferFormat } from "../src/ITransport";
 import { NullLogger } from "../src/Loggers";
+import { ConsoleLogger } from "../src/Utils";
 
 import { VerifyLogger } from "./Common";
 import { TestHttpClient } from "./TestHttpClient";
@@ -26,15 +27,51 @@ const commonHttpOptions: IHttpConnectionOptions = {
     logMessageContent: true,
 };
 
+// We use a different mapping table here to help catch any unintentional breaking changes.
+// tslint:disable:object-literal-sort-keys
+const ExpectedLogLevelMappings = {
+    trace: LogLevel.Trace,
+    debug: LogLevel.Debug,
+    info: LogLevel.Information,
+    information: LogLevel.Information,
+    warn: LogLevel.Warning,
+    warning: LogLevel.Warning,
+    error: LogLevel.Error,
+    critical: LogLevel.Critical,
+    none: LogLevel.None,
+};
+
+class CapturingConsole {
+    public messages: any[] = [];
+
+    public error(message: any) {
+        this.messages.push(CapturingConsole.stripPrefix(message));
+    }
+
+    public warn(message: any) {
+        this.messages.push(CapturingConsole.stripPrefix(message));
+    }
+
+    public info(message: any) {
+        this.messages.push(CapturingConsole.stripPrefix(message));
+    }
+
+    public log(message: any) {
+        this.messages.push(CapturingConsole.stripPrefix(message));
+    }
+
+    private static stripPrefix(input: any): any {
+        if (typeof input === "string") {
+            input = input.replace(/\[.*\]\s+/, "");
+        }
+        return input;
+    }
+}
+
 registerUnhandledRejectionHandler();
 
 describe("HubConnectionBuilder", () => {
     eachMissingValue((val, name) => {
-        it(`configureLogging throws if logger is ${name}`, () => {
-            const builder = new HubConnectionBuilder();
-            expect(() => builder.configureLogging(val!)).toThrow("The 'logging' argument is required.");
-        });
-
         it(`withUrl throws if url is ${name}`, () => {
             const builder = new HubConnectionBuilder();
             expect(() => builder.withUrl(val!)).toThrow("The 'url' argument is required.");
@@ -111,98 +148,160 @@ describe("HubConnectionBuilder", () => {
         });
     });
 
-    it("allows logger to be replaced", async () => {
+    describe("configureLogging", async () => {
-        let loggedMessages = 0;
+        function testLogLevels(logger: ILogger, minLevel: LogLevel) {
-        const logger = {
+            const capturingConsole = new CapturingConsole();
-            log() {
+            (logger as ConsoleLogger).outputConsole = capturingConsole;
-                loggedMessages += 1;
-            },
-        };
-        const pollSent = new PromiseSource<HttpRequest>();
-        const pollCompleted = new PromiseSource<HttpResponse>();
-        const testClient = createTestClient(pollSent, pollCompleted.promise)
-            .on("POST", "http://example.com?id=abc123", (req) => {
-                // Respond from the poll with the handshake response
-                pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
-                return new HttpResponse(202);
-            });
-        const connection = createConnectionBuilder(logger)
-            .withUrl("http://example.com", {
-                ...commonHttpOptions,
-                httpClient: testClient,
-            })
-            .build();
 
-        try {
+            for (let level = LogLevel.Trace; level < LogLevel.None; level++) {
-            await connection.start();
+                const message = `Message at LogLevel.${LogLevel[level]}`;
-        } catch {
+                const expectedMessage = `${LogLevel[level]}: Message at LogLevel.${LogLevel[level]}`;
-            // Ignore failures
+                logger.log(level, message);
+
+                if (level >= minLevel) {
+                    expect(capturingConsole.messages).toContain(expectedMessage);
+                } else {
+                    expect(capturingConsole.messages).not.toContain(expectedMessage);
+                }
+            }
         }
 
-        expect(loggedMessages).toBeGreaterThan(0);
+        eachMissingValue((val, name) => {
-    });
+            it(`throws if logger is ${name}`, () => {
-
+                const builder = new HubConnectionBuilder();
-    it("uses logger for both HttpConnection and HubConnection", async () => {
+                expect(() => builder.configureLogging(val!)).toThrow("The 'logging' argument is required.");
-        const pollSent = new PromiseSource<HttpRequest>();
-        const pollCompleted = new PromiseSource<HttpResponse>();
-        const testClient = createTestClient(pollSent, pollCompleted.promise)
-            .on("POST", "http://example.com?id=abc123", (req) => {
-                // Respond from the poll with the handshake response
-                pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
-                return new HttpResponse(202);
             });
-        const logger = new CaptureLogger();
+        });
-        const connection = createConnectionBuilder(logger)
-            .withUrl("http://example.com", {
-                ...commonHttpOptions,
-                httpClient: testClient,
-            })
-            .build();
 
-        try {
+        [
-            await connection.start();
+            LogLevel.None,
-        } catch {
+            LogLevel.Critical,
-            // Ignore failures
+            LogLevel.Error,
+            LogLevel.Warning,
+            LogLevel.Information,
+            LogLevel.Debug,
+            LogLevel.Trace,
+        ].forEach((minLevel) => {
+            const levelName = LogLevel[minLevel];
+            it(`accepts LogLevel.${levelName}`, async () => {
+                const builder = new HubConnectionBuilder()
+                    .configureLogging(minLevel);
+
+                expect(builder.logger).toBeDefined();
+                expect(builder.logger).toBeInstanceOf(ConsoleLogger);
+
+                testLogLevels(builder.logger!, minLevel);
+            });
+        });
+
+        const levelNames = Object.keys(ExpectedLogLevelMappings) as Array<keyof typeof ExpectedLogLevelMappings>;
+        for (const str of levelNames) {
+            const mapped = ExpectedLogLevelMappings[str];
+            const mappedName = LogLevel[mapped];
+            it(`accepts "${str}" as an alias for LogLevel.${mappedName}`, async () => {
+                const builder = new HubConnectionBuilder()
+                    .configureLogging(str);
+
+                expect(builder.logger).toBeDefined();
+                expect(builder.logger).toBeInstanceOf(ConsoleLogger);
+
+                testLogLevels(builder.logger!, mapped);
+            });
         }
 
-        // A HubConnection message
+        it("allows logger to be replaced", async () => {
-        expect(logger.messages).toContain("Starting HubConnection.");
+            let loggedMessages = 0;
+            const logger = {
+                log() {
+                    loggedMessages += 1;
+                },
+            };
+            const pollSent = new PromiseSource<HttpRequest>();
+            const pollCompleted = new PromiseSource<HttpResponse>();
+            const testClient = createTestClient(pollSent, pollCompleted.promise)
+                .on("POST", "http://example.com?id=abc123", (req) => {
+                    // Respond from the poll with the handshake response
+                    pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
+                    return new HttpResponse(202);
+                });
+            const connection = createConnectionBuilder(logger)
+                .withUrl("http://example.com", {
+                    ...commonHttpOptions,
+                    httpClient: testClient,
+                })
+                .build();
 
-        // An HttpConnection message
+            try {
-        expect(logger.messages).toContain("Starting connection with transfer format 'Text'.");
+                await connection.start();
-    });
+            } catch {
+                // Ignore failures
+            }
 
-    it("does not replace HttpConnectionOptions logger if provided", async () => {
+            expect(loggedMessages).toBeGreaterThan(0);
-        const pollSent = new PromiseSource<HttpRequest>();
+        });
-        const pollCompleted = new PromiseSource<HttpResponse>();
-        const testClient = createTestClient(pollSent, pollCompleted.promise)
-            .on("POST", "http://example.com?id=abc123", (req) => {
-                // Respond from the poll with the handshake response
-                pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
-                return new HttpResponse(202);
-            });
-        const hubConnectionLogger = new CaptureLogger();
-        const httpConnectionLogger = new CaptureLogger();
-        const connection = createConnectionBuilder(hubConnectionLogger)
-            .withUrl("http://example.com", {
-                httpClient: testClient,
-                logger: httpConnectionLogger,
-            })
-            .build();
 
-        try {
+        it("configures logger for both HttpConnection and HubConnection", async () => {
-            await connection.start();
+            const pollSent = new PromiseSource<HttpRequest>();
-        } catch {
+            const pollCompleted = new PromiseSource<HttpResponse>();
-            // Ignore failures
+            const testClient = createTestClient(pollSent, pollCompleted.promise)
-        }
+                .on("POST", "http://example.com?id=abc123", (req) => {
+                    // Respond from the poll with the handshake response
+                    pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
+                    return new HttpResponse(202);
+                });
+            const logger = new CaptureLogger();
+            const connection = createConnectionBuilder(logger)
+                .withUrl("http://example.com", {
+                    ...commonHttpOptions,
+                    httpClient: testClient,
+                })
+                .build();
 
-        // A HubConnection message
+            try {
-        expect(hubConnectionLogger.messages).toContain("Starting HubConnection.");
+                await connection.start();
-        expect(httpConnectionLogger.messages).not.toContain("Starting HubConnection.");
+            } catch {
+                // Ignore failures
+            }
 
-        // An HttpConnection message
+            // A HubConnection message
-        expect(httpConnectionLogger.messages).toContain("Starting connection with transfer format 'Text'.");
+            expect(logger.messages).toContain("Starting HubConnection.");
-        expect(hubConnectionLogger.messages).not.toContain("Starting connection with transfer format 'Text'.");
+
+            // An HttpConnection message
+            expect(logger.messages).toContain("Starting connection with transfer format 'Text'.");
+        });
+
+        it("does not replace HttpConnectionOptions logger if provided", async () => {
+            const pollSent = new PromiseSource<HttpRequest>();
+            const pollCompleted = new PromiseSource<HttpResponse>();
+            const testClient = createTestClient(pollSent, pollCompleted.promise)
+                .on("POST", "http://example.com?id=abc123", (req) => {
+                    // Respond from the poll with the handshake response
+                    pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
+                    return new HttpResponse(202);
+                });
+            const hubConnectionLogger = new CaptureLogger();
+            const httpConnectionLogger = new CaptureLogger();
+            const connection = createConnectionBuilder(hubConnectionLogger)
+                .withUrl("http://example.com", {
+                    httpClient: testClient,
+                    logger: httpConnectionLogger,
+                })
+                .build();
+
+            try {
+                await connection.start();
+            } catch {
+                // Ignore failures
+            }
+
+            // A HubConnection message
+            expect(hubConnectionLogger.messages).toContain("Starting HubConnection.");
+            expect(httpConnectionLogger.messages).not.toContain("Starting HubConnection.");
+
+            // An HttpConnection message
+            expect(httpConnectionLogger.messages).toContain("Starting connection with transfer format 'Text'.");
+            expect(hubConnectionLogger.messages).not.toContain("Starting connection with transfer format 'Text'.");
+        });
     });
 
     it("reconnectPolicy undefined by default", () => {
@@ -228,7 +327,7 @@ describe("HubConnectionBuilder", () => {
     });
 
     it("withAutomaticReconnect uses custom retryDelays when provided", () => {
-        const customRetryDelays = [ 3, 1, 4, 1, 5, 9 ];
+        const customRetryDelays = [3, 1, 4, 1, 5, 9];
         const builder = new HubConnectionBuilder()
             .withAutomaticReconnect(customRetryDelays);
 
@@ -241,7 +340,7 @@ describe("HubConnectionBuilder", () => {
     });
 
     it("withAutomaticReconnect uses a custom IReconnectPolicy when provided", () => {
-        const customRetryDelays = [ 127, 0, 0, 1 ];
+        const customRetryDelays = [127, 0, 0, 1];
         const builder = new HubConnectionBuilder()
             .withAutomaticReconnect(new DefaultReconnectPolicy(customRetryDelays));
 

src/SignalR/samples/SignalRSamples/wwwroot/hubs.html

@@ -151,7 +151,7 @@
         console.log('http://' + document.location.host + '/' + hubRoute);
 
         var connectionBuilder = new signalR.HubConnectionBuilder()
-            .configureLogging(signalR.LogLevel.Trace)
+            .configureLogging("trace")
             .withUrl(hubRoute, options)
             .withHubProtocol(protocol);