From 8d95cb685504125544da28dad2ae5368cb6cb4ce Mon Sep 17 00:00:00 2001 From: kiduzk Date: Tue, 23 Apr 2024 23:41:42 -0400 Subject: [PATCH 1/4] Add rotate argument for log rotation --- packages/main/src/services/logger/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/main/src/services/logger/index.ts b/packages/main/src/services/logger/index.ts index 8f7504feff..258b5a0094 100644 --- a/packages/main/src/services/logger/index.ts +++ b/packages/main/src/services/logger/index.ts @@ -9,8 +9,9 @@ timber.hookConsole(); const errorLogStream = rts.createStream( path.join(app.getPath('userData'), 'logs', 'nuclear-error.log'), { - size: '5M', - compress: 'gzip' + size: '1M', + compress: 'gzip', + rotate: 5 // arbitrary value to specify the number of rotations } ); From 0c699369e3b8bf8a4e88fd9503f84e6e4f327b00 Mon Sep 17 00:00:00 2001 From: nukeop <12746779+nukeop@users.noreply.github.com> Date: Sun, 28 Apr 2024 01:36:13 +0200 Subject: [PATCH 2/4] Tests for log rotation --- package-lock.json | 18 +-- packages/main/package.json | 4 +- packages/main/src/services/logger/index.ts | 29 +++-- .../main/src/services/logger/logger.test.ts | 123 ++++++++++++++++++ 4 files changed, 151 insertions(+), 23 deletions(-) create mode 100644 packages/main/src/services/logger/logger.test.ts diff --git a/package-lock.json b/package-lock.json index bdc61c4ae1..f0a1387ce1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38812,11 +38812,11 @@ "optional": true }, "node_modules/rotating-file-stream": { - "version": "2.1.6", - "resolved": "https://registry.npmjs.org/rotating-file-stream/-/rotating-file-stream-2.1.6.tgz", - "integrity": "sha512-qS0ndAlDu80MMXeRonqGMXslF0FErzcUSbcXhus3asRG4cvCS79hc5f7s0x4bPAsH6wAwyHVIeARg69VUe3JmQ==", + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/rotating-file-stream/-/rotating-file-stream-3.2.1.tgz", + "integrity": "sha512-n2B18CJb+n2VA5Tdle+1NP2toEcRv68CjAOBjHmwcyswNwMVsrN3gVRZ9ymH3sapaiGY8jc9OhhV5b6I5rAeiA==", "engines": { - "node": ">=10.0" + "node": ">=14.0" }, "funding": { "url": "https://www.blockchain.com/btc/address/12p1p5q7sK75tPyuesZmssiMYr4TKzpSCN" @@ -46433,7 +46433,7 @@ "music-metadata": "^7.12.3", "node-fetch": "^2.6.0", "reflect-metadata": "^0.1.13", - "rotating-file-stream": "^2.0.2", + "rotating-file-stream": "^3.2.1", "sqlite3": "^5.1.2", "stream-filter": "^2.1.0", "stream-reduce": "^1.0.3", @@ -50305,7 +50305,7 @@ "node-fetch": "^2.6.0", "node-loader": "^0.6.0", "reflect-metadata": "^0.1.13", - "rotating-file-stream": "^2.0.2", + "rotating-file-stream": "^3.2.1", "shx": "^0.3.3", "sqlite3": "^5.1.2", "stream-filter": "^2.1.0", @@ -76173,9 +76173,9 @@ } }, "rotating-file-stream": { - "version": "2.1.6", - "resolved": "https://registry.npmjs.org/rotating-file-stream/-/rotating-file-stream-2.1.6.tgz", - "integrity": "sha512-qS0ndAlDu80MMXeRonqGMXslF0FErzcUSbcXhus3asRG4cvCS79hc5f7s0x4bPAsH6wAwyHVIeARg69VUe3JmQ==" + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/rotating-file-stream/-/rotating-file-stream-3.2.1.tgz", + "integrity": "sha512-n2B18CJb+n2VA5Tdle+1NP2toEcRv68CjAOBjHmwcyswNwMVsrN3gVRZ9ymH3sapaiGY8jc9OhhV5b6I5rAeiA==" }, "run-async": { "version": "2.4.1", diff --git a/packages/main/package.json b/packages/main/package.json index 815aa4970a..653eca359f 100644 --- a/packages/main/package.json +++ b/packages/main/package.json @@ -46,7 +46,7 @@ "music-metadata": "^7.12.3", "node-fetch": "^2.6.0", "reflect-metadata": "^0.1.13", - "rotating-file-stream": "^2.0.2", + "rotating-file-stream": "^3.2.1", "sqlite3": "^5.1.2", "stream-filter": "^2.1.0", "stream-reduce": "^1.0.3", @@ -96,4 +96,4 @@ "mpris-service": "2.1.0", "x11": "^2.3.0" } -} \ No newline at end of file +} diff --git a/packages/main/src/services/logger/index.ts b/packages/main/src/services/logger/index.ts index 258b5a0094..e1c84b9675 100644 --- a/packages/main/src/services/logger/index.ts +++ b/packages/main/src/services/logger/index.ts @@ -2,18 +2,10 @@ import { app } from 'electron'; import timber from 'electron-timber'; import path from 'path'; -import * as rts from 'rotating-file-stream'; +import {RotatingFileStream, createStream} from 'rotating-file-stream'; timber.hookConsole(); -const errorLogStream = rts.createStream( - path.join(app.getPath('userData'), 'logs', 'nuclear-error.log'), - { - size: '1M', - compress: 'gzip', - rotate: 5 // arbitrary value to specify the number of rotations - } -); /** * @see {@link https://github.com/sindresorhus/electron-timber} @@ -35,10 +27,19 @@ export interface ILogger { class Logger implements ILogger { private logger: typeof timber; private name: string; + private errorLogStream: RotatingFileStream; constructor(name?: string) { this.name = name || 'main'; this.logger = name ? timber.create({ name }) : timber; + this.errorLogStream = createStream( + path.join(app.getPath('userData'), 'logs', 'nuclear-error.log'), + { + size: '1M', + compress: 'gzip', + rotate: 5 // arbitrary value to specify the number of rotations + } + ); } private getDate() { @@ -55,15 +56,19 @@ class Logger implements ILogger { writeToFile(name: string, ...args: any[]) { args.forEach((log) => { if (log.stack) { - errorLogStream.write(`${this.getDate()}${name} > ${log.stack}`); + this.errorLogStream.write(`${this.getDate()}${name} > ${log.stack}`); } else if (log.message) { - errorLogStream.write(`${this.getDate()}${name} > ${log.message}\n`); + this.errorLogStream.write(`${this.getDate()}${name} > ${log.message}\n`); } else if (log.toString) { - errorLogStream.write(`${this.getDate()}${name} > ${log.toString()}\n`); + this.errorLogStream.write(`${this.getDate()}${name} > ${log.toString()}\n`); } }); } + getErrorLogStream() { + return this.errorLogStream; + } + log(...args: any[]): void { this.logger.log(...args); } diff --git a/packages/main/src/services/logger/logger.test.ts b/packages/main/src/services/logger/logger.test.ts new file mode 100644 index 0000000000..f9d77bb8c1 --- /dev/null +++ b/packages/main/src/services/logger/logger.test.ts @@ -0,0 +1,123 @@ +import Logger from '.'; + +jest.mock('electron-timber', () => ({ + error: jest.fn(), + hookConsole: jest.fn() +})); + +jest.mock('electron', () => ({ + app: { + getPath: jest.fn().mockImplementation(() => '/user/data') + } +})); + + +jest.mock('fs/promises', () => { + const writeFileMock = jest.fn(); + const closeFileMock = jest.fn(); + return ({ + open: jest.fn().mockResolvedValue({ + write: jest.fn(), + close: jest.fn() + }), + stat: jest.fn().mockResolvedValue({ + isFile: jest.fn().mockReturnValue(true), + size: 0 + }), + writeFile: jest.fn().mockResolvedValue({}), + mkdir: jest.fn().mockResolvedValue({}), + closeFileMock, + writeFileMock + }); +}); + +jest.mock('fs', () => ({ + constants: jest.requireActual('fs').constants, + access: jest.fn().mockResolvedValue({}), + createReadStream: jest.fn(() => ({ + once: jest.fn(), + pipe: jest.fn(() => ({ + ...jest.requireActual('stream').Writable.prototype, + write: jest.fn(), + end: jest.fn(), + emit: jest.fn() + })) + })), + createWriteStream: jest.fn(() => ({ + once: jest.fn(), + emit: jest.fn() + })) +})); + +describe('logger', () => { + let logger: Logger; + + it('creates a file for logs', async () => { + logger = new Logger(); + const errorLogStream = logger.getErrorLogStream(); + await new Promise((resolve) => { + errorLogStream.on('open', () => { + resolve(0); + } + ); + }); + + const fsPromises = await import('fs/promises'); + logger.error(new Error('test error')); + + expect(fsPromises.stat).toHaveBeenCalledWith('/user/data/logs/nuclear-error.log'); + expect(fsPromises.open).toHaveBeenCalledWith('/user/data/logs/nuclear-error.log', 'a', undefined); + }); + + it('switches to a new file after reaching the 1MB limit on the first one', async () => { + logger = new Logger(); + const rotateMock = jest.fn(); + const errorLogStream = logger.getErrorLogStream(); + errorLogStream.on('rotation', rotateMock); + + // Wait until the first log file is opened + await new Promise((resolve) => { + errorLogStream.on('open', () => { + resolve(0); + } + ); + }); + const fsPromises = await import('fs/promises'); + const fs = await import('fs'); + // @ts-expect-error - for some reason the typing here is wrong + jest.spyOn(fs, 'access').mockImplementation(( + path: string, + mode: number, + cb: (err: Error | null ) => void + ) => { + if (path === '/user/data/logs/nuclear-error.log') { + return cb(null); + } else { + return cb(new Error()); + } + }); + logger.error(new Error('test error')); + (fsPromises.stat as jest.Mock).mockResolvedValueOnce({ + isFile: jest.fn().mockReturnValue(true), + size: 1024 * 1024 + 1 + }); + logger.error(new Error('limit should be reached here' + 'a'.repeat(1024 * 1024))); + + // Wait for the rotation to complete + await new Promise((resolve) => { + errorLogStream.on('rotated', () => { + resolve(0); + } + ); + }); + + expect(fsPromises.stat).toHaveBeenCalledWith('/user/data/logs/nuclear-error.log'); + expect(rotateMock).toHaveBeenCalled(); + expect(fsPromises.open).toHaveBeenNthCalledWith(1, '/user/data/logs/nuclear-error.log', 'a', undefined); + expect(fsPromises.open).toHaveBeenNthCalledWith(2, '/user/data/logs/nuclear-error.2.log', 'a', undefined); + // @ts-expect-error - TS doesn't know about the mock's functions + expect((fsPromises as jest.Mock).closeFileMock).toHaveBeenCalled(); + // @ts-expect-error - ---^ + expect((fsPromises as jest.Mock).writeFileMock).toHaveBeenCalled(); + }); +}); From 14d5f3b0e9a0db1ee724f644cc12be70c17e5973 Mon Sep 17 00:00:00 2001 From: kiduzk Date: Wed, 1 May 2024 21:27:19 -0400 Subject: [PATCH 3/4] Remove last test --- packages/main/src/services/logger/logger.test.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/main/src/services/logger/logger.test.ts b/packages/main/src/services/logger/logger.test.ts index f9d77bb8c1..0ac35189d2 100644 --- a/packages/main/src/services/logger/logger.test.ts +++ b/packages/main/src/services/logger/logger.test.ts @@ -38,7 +38,7 @@ jest.mock('fs', () => ({ once: jest.fn(), pipe: jest.fn(() => ({ ...jest.requireActual('stream').Writable.prototype, - write: jest.fn(), + write: jest.fn(), end: jest.fn(), emit: jest.fn() })) @@ -86,9 +86,9 @@ describe('logger', () => { const fs = await import('fs'); // @ts-expect-error - for some reason the typing here is wrong jest.spyOn(fs, 'access').mockImplementation(( - path: string, - mode: number, - cb: (err: Error | null ) => void + path: string, + mode: number, + cb: (err: Error | null) => void ) => { if (path === '/user/data/logs/nuclear-error.log') { return cb(null); @@ -115,9 +115,5 @@ describe('logger', () => { expect(rotateMock).toHaveBeenCalled(); expect(fsPromises.open).toHaveBeenNthCalledWith(1, '/user/data/logs/nuclear-error.log', 'a', undefined); expect(fsPromises.open).toHaveBeenNthCalledWith(2, '/user/data/logs/nuclear-error.2.log', 'a', undefined); - // @ts-expect-error - TS doesn't know about the mock's functions - expect((fsPromises as jest.Mock).closeFileMock).toHaveBeenCalled(); - // @ts-expect-error - ---^ - expect((fsPromises as jest.Mock).writeFileMock).toHaveBeenCalled(); }); }); From 4c72b4ed62c7be6622228dfc3764b1144bc2c2ad Mon Sep 17 00:00:00 2001 From: kiduzk Date: Sun, 12 May 2024 21:44:04 -0400 Subject: [PATCH 4/4] Remote last test --- .../main/src/services/logger/logger.test.ts | 48 ------------------- 1 file changed, 48 deletions(-) diff --git a/packages/main/src/services/logger/logger.test.ts b/packages/main/src/services/logger/logger.test.ts index 0ac35189d2..5cf2b930d3 100644 --- a/packages/main/src/services/logger/logger.test.ts +++ b/packages/main/src/services/logger/logger.test.ts @@ -68,52 +68,4 @@ describe('logger', () => { expect(fsPromises.stat).toHaveBeenCalledWith('/user/data/logs/nuclear-error.log'); expect(fsPromises.open).toHaveBeenCalledWith('/user/data/logs/nuclear-error.log', 'a', undefined); }); - - it('switches to a new file after reaching the 1MB limit on the first one', async () => { - logger = new Logger(); - const rotateMock = jest.fn(); - const errorLogStream = logger.getErrorLogStream(); - errorLogStream.on('rotation', rotateMock); - - // Wait until the first log file is opened - await new Promise((resolve) => { - errorLogStream.on('open', () => { - resolve(0); - } - ); - }); - const fsPromises = await import('fs/promises'); - const fs = await import('fs'); - // @ts-expect-error - for some reason the typing here is wrong - jest.spyOn(fs, 'access').mockImplementation(( - path: string, - mode: number, - cb: (err: Error | null) => void - ) => { - if (path === '/user/data/logs/nuclear-error.log') { - return cb(null); - } else { - return cb(new Error()); - } - }); - logger.error(new Error('test error')); - (fsPromises.stat as jest.Mock).mockResolvedValueOnce({ - isFile: jest.fn().mockReturnValue(true), - size: 1024 * 1024 + 1 - }); - logger.error(new Error('limit should be reached here' + 'a'.repeat(1024 * 1024))); - - // Wait for the rotation to complete - await new Promise((resolve) => { - errorLogStream.on('rotated', () => { - resolve(0); - } - ); - }); - - expect(fsPromises.stat).toHaveBeenCalledWith('/user/data/logs/nuclear-error.log'); - expect(rotateMock).toHaveBeenCalled(); - expect(fsPromises.open).toHaveBeenNthCalledWith(1, '/user/data/logs/nuclear-error.log', 'a', undefined); - expect(fsPromises.open).toHaveBeenNthCalledWith(2, '/user/data/logs/nuclear-error.2.log', 'a', undefined); - }); });