Merge branch 'improve-logger'

5 files changed, 63 insertions, 8 deletions

diff --git a/gui/package.json b/gui/package.json
index 3bcdf71167..026ca36e28 100644
--- a/gui/package.json
+++ b/gui/package.json
@@ -29,8 +29,7 @@
     "redux": "^4.0.5",
     "sprintf-js": "^1.1.2",
     "styled-components": "^5.1.1",
-    "uuid": "^3.0.1",
-    "nseventmonitor": "^1.0.0"
+    "uuid": "^3.0.1"
   },
   "optionalDependencies": {
     "nseventmonitor": "^1.0.0"
diff --git a/gui/src/main/logging.ts b/gui/src/main/logging.ts
index fb3730d59e..23c0f6ad1d 100644
--- a/gui/src/main/logging.ts
+++ b/gui/src/main/logging.ts
@@ -4,7 +4,7 @@ import path from 'path';
 import { IpcMainEventChannel } from './ipc-event-channel';
 import { LogLevel, ILogInput, ILogOutput } from '../shared/logging-types';
 
-export const OLD_LOG_FILES = ['frontend-renderer.log'];
+export const OLD_LOG_FILES = ['main.log', 'renderer.log'];
 
 export class FileOutput implements ILogOutput {
   private fileDescriptor: number;
@@ -37,11 +37,11 @@ export class IpcInput implements ILogInput {
 }
 
 export function getMainLogPath() {
-  return path.join(getLogDirectoryDir(), 'main.log');
+  return path.join(getLogDirectoryDir(), 'frontend-main.log');
 }
 
 export function getRendererLogPath() {
-  return path.join(getLogDirectoryDir(), 'renderer.log');
+  return path.join(getLogDirectoryDir(), 'frontend-renderer.log');
 }
 
 export function createLoggingDirectory(): void {
diff --git a/gui/src/shared/logging.ts b/gui/src/shared/logging.ts
index 43d0b271d9..84eaa5f25c 100644
--- a/gui/src/shared/logging.ts
+++ b/gui/src/shared/logging.ts
@@ -14,7 +14,8 @@ export class Logger {
 
   public log(level: LogLevel, ...data: unknown[]) {
     const time = moment().format('YYYY-MM-DD HH:mm:ss.SSS');
-    const message = `[${time}][${LogLevel[level]}] ${data.join(' ')}`;
+    const stringifiedData = data.map(this.stringifyData).join(' ');
+    const message = `[${time}][${LogLevel[level]}] ${stringifiedData}`;
 
     this.outputMessage(level, message);
   }
@@ -29,6 +30,10 @@ export class Logger {
     this.outputs.forEach((output) => output.dispose?.());
   }
 
+  private stringifyData(data: unknown): string {
+    return typeof data === 'string' ? data : JSON.stringify(data);
+  }
+
   private outputMessage(level: LogLevel, message: string) {
     this.outputs
       .filter((output) => level <= output.level)
diff --git a/gui/test/ip.spec.ts b/gui/test/ip.spec.ts
index b430e4d2b3..3a86c5891c 100644
--- a/gui/test/ip.spec.ts
+++ b/gui/test/ip.spec.ts
@@ -76,7 +76,7 @@ const publicIpAddresses = [
   '192.169.0.0',
 ];
 
-describe('Logging', () => {
+describe('IP', () => {
   it('should detect that valid IPv4 addresses are valid', () => {
     validIpv4Addresses.forEach((ipAddress) => {
       const valid = ip.IPv4Address.isValid(ipAddress);
diff --git a/gui/test/logging.spec.ts b/gui/test/logging.spec.ts
index 22c3db02fa..7d4edf386f 100644
--- a/gui/test/logging.spec.ts
+++ b/gui/test/logging.spec.ts
@@ -1,8 +1,10 @@
-import { expect } from 'chai';
+import { expect, spy } from 'chai';
 import fs from 'fs';
 import sinon from 'sinon';
 import { it, describe, before, beforeEach, after } from 'mocha';
+import { Logger } from '../src/shared/logging';
 import { backupLogFile, rotateOrDeleteFile } from '../src/main/logging';
+import { LogLevel } from '../src/shared/logging-types';
 
 const aPath = 'log-directory/a.log';
 const oldAPath = 'log-directory/a.old.log';
@@ -78,4 +80,53 @@ describe('Logging', () => {
     expect(fs.accessSync.bind(null, bPath)).to.throw();
     expect(fs.accessSync.bind(null, oldBPath)).to.throw();
   });
+
+  it('should only log for the correct log level', () => {
+    const logger = new Logger();
+
+    const errorSpy = spy();
+    const warningSpy = spy();
+    const infoSpy = spy();
+    const verboseSpy = spy();
+    const debugSpy = spy();
+
+    logger.addOutput({ level: LogLevel.error, write: errorSpy });
+    logger.addOutput({ level: LogLevel.warning, write: warningSpy });
+    logger.addOutput({ level: LogLevel.info, write: infoSpy });
+    logger.addOutput({ level: LogLevel.verbose, write: verboseSpy });
+    logger.addOutput({ level: LogLevel.debug, write: debugSpy });
+
+    logger.error();
+    logger.warn();
+    logger.info();
+    logger.verbose();
+    logger.debug();
+
+    expect(errorSpy).to.have.been.called.exactly(1);
+    expect(warningSpy).to.have.been.called.exactly(2);
+    expect(infoSpy).to.have.been.called.exactly(3);
+    expect(verboseSpy).to.have.been.called.exactly(4);
+    expect(debugSpy).to.have.been.called.exactly(5);
+  });
+
+  it('should format JavaScript types correctly', async () => {
+    const logger = new Logger();
+
+    const promise = new Promise((resolve, _reject) => {
+      logger.addOutput({
+        level: LogLevel.info,
+        write: (_, message) => resolve(message.replace(/^.*\[info\] /, '')),
+      });
+    });
+
+    logger.info('zero', 'one two', 3, [4, 5, 'six', { seven: 'eight' }], {
+      nine: 10,
+      eleven: [true],
+    });
+    await expect(promise).to.eventually.be.fulfilled.then((result) => {
+      expect(result).to.equal(
+        'zero one two 3 [4,5,"six",{"seven":"eight"}] {"nine":10,"eleven":[true]}',
+      );
+    });
+  });
 });