Bug fixes

Author: sam-lord

index.js

@@ -21,7 +21,7 @@ class RotatingFileStream {
         this._triggers = [];
         if (config.period) {
             const periodTrigger = new PeriodTrigger(config.period);
-            this.triggers.push(periodTrigger);
+            this._triggers.push(periodTrigger);
             if (config.rotateExisting) {
                 this._rotator.once(FileStartTime, (startTime) => {
                     periodTrigger.setInitialTime(startTime);
@@ -35,7 +35,9 @@ class RotatingFileStream {
         }
         this._rotatingLock = false;
         this._triggers.forEach((trigger) => {
-            trigger.on(Rotate, this._rotate);
+            trigger.on(Rotate, () => {
+                this._rotate();
+            });
         });
         this._rotator.on(NewFile, () => {
             this._queue.setFileHandle(this._rotator.getCurrentHandle());
@@ -52,7 +54,7 @@ class RotatingFileStream {
         this._rotatingLock = true;
         await this._queue.pause();
         const nextFileHandle = await this._rotator.rotate();
-        this._triggers.forEach(trigger => trigger.rotated());
+        this._triggers.forEach(trigger => trigger.newFile());
         this._queue.setFileHandle(nextFileHandle);
         this._rotatingLock = false;
     }

lib/fileRotator.js

@@ -5,6 +5,7 @@ const {createReadStream, createWriteStream} = require('fs');
 const fs = require('fs/promises');
 const pipe = promisify(pipeline);
 const path = require('path');
+const {processSize} = require('../util/configProcessors');
 
 const {EventEmitter} = require('events');
 const {NewFile, FileStartTime} = require('./customEvents');
@@ -15,6 +16,8 @@ class FileRotator extends EventEmitter {
     constructor(config) {
         super();
         this._startNewFile = config.startNewFile;
+        this._totalFiles = config.totalFiles;
+        this._totalSize = processSize(config.totalSize);
         this._gzip = config.gzip;
         this._path = config.path;
         this._initialised = false;
@@ -49,8 +52,8 @@ class FileRotator extends EventEmitter {
         if (this._gzip) {
             await this._gzipCurrentFile();
         }
-        await this._moveIntermediateFiles();
         await this._deleteFiles();
+        await this._moveIntermediateFiles();
         await this._initialiseNewFile();
         // Set by _initialiseNewFile
         return this._currentHandle;
@@ -113,13 +116,34 @@ class FileRotator extends EventEmitter {
     }
 
     async _deleteFiles() {
-        
+        const filesToDelete = [];
+        let fileNumber = 0;
+        let bytesTotal = 0;
+        for (; ; fileNumber++) {
+            const name = this._getFileName(fileNumber, true);
+            try {
+                const result = await fs.stat(name);
+                bytesTotal += result.size;
+                if ((this._totalSize && bytesTotal > this._totalSize) || (this._totalFiles && fileNumber + 1 > this._totalFiles)) {
+                    filesToDelete.push(name);
+                }
+            } catch (err) {
+                if (err.code === 'ENOENT') {
+                    break;
+                } else {
+                    throw err;
+                }
+            }
+        }
+        for (const file of filesToDelete) {
+            await fs.unlink(file);
+        }
     }
 
     async _moveIntermediateFiles() {
         const filesToMove = [];
         let fileNumber = 0;
-        for (;;) {
+        for (; ; fileNumber++) {
             const name = this._getFileName(fileNumber, true);
             try {
                 const result = await fs.stat(name);
@@ -131,7 +155,6 @@ class FileRotator extends EventEmitter {
                     throw err;
                 }
             }
-            fileNumber += 1;
         }
         // Set to last file in sequence
         fileNumber -= 1;

lib/periodTrigger.js

@@ -1,30 +1,6 @@
 const {EventEmitter} = require('events');
 const {Rotate} = require('./customEvents');
-
-const parsePeriod = (period) => {
-    const result = {
-        num: 1,
-        unit: 'd'
-    };
-
-    // Parse `period`.
-    if (period) {
-        var crackedperiod = {
-            hourly: '1h',
-            daily: '1d',
-            weekly: '1w',
-            monthly: '1m',
-            yearly: '1y'
-        }[period] || period;
-
-        const [num, unit] = /^([1-9][0-9]*)([hdwmy]|ms)$/.exec(crackedperiod);
-
-        result.num = Number(num);
-        result.unit = unit;
-    }
-
-    return result;
-};
+const {processPeriod} = require('../util/configProcessors');
 
 const getNextRotation = (period, lastRotation) => {
     var date = new Date();
@@ -94,7 +70,7 @@ const getNextRotation = (period, lastRotation) => {
 class PeriodTrigger extends EventEmitter {
     constructor(period) {
         super();
-        this._period = parsePeriod(period);
+        this._period = processPeriod(period);
         this._task = null;
         this._rotateAt = null;
     }

lib/thresholdTrigger.js

@@ -1,26 +1,11 @@
 const {EventEmitter} = require('events');
 const {Rotate} = require('./customEvents');
-
-const processThreshold = (threshold) => {
-    const multipliers = {
-        b: 1,
-        k: 1024,
-        m: 1024 * 1024,
-        g: 1024 * 1024 * 1024
-    };
-    
-    if (typeof (threshold) === 'string') {
-        const [num, unit] = /^([1-9][0-9]*)([bkmg])$/.exec(threshold).slice(1);
-        return Number(num) * multipliers[unit];
-    } else {
-        return threshold;
-    }
-};
+const {processSize} = require('../util/configProcessors');
 
 class ThresholdTrigger extends EventEmitter {
     constructor(threshold) {
         super();
-        this._threshold = processThreshold(threshold);
+        this._threshold = processSize(threshold);
         this._totalWritten = 0;
     }
 

test/filerotate.test.js

@@ -5,7 +5,7 @@ const bunyan = require('bunyan');
 
 const RotatingFileStream = require('../index');
 const testConfig = {
-    path: 'foo.log',
+    path: 'logs/foo.log',
     totalFiles: 10,
     threshold: '1k',
     rotateExisting: true,

util/configProcessors.js

@@ -0,0 +1,45 @@
+const processSize = (size) => {
+    const multipliers = {
+        b: 1,
+        k: 1024,
+        m: 1024 * 1024,
+        g: 1024 * 1024 * 1024
+    };
+    
+    if (typeof (size) === 'string') {
+        const [num, unit] = /^([1-9][0-9]*)([bkmg])$/.exec(size).slice(1);
+        return Number(num) * multipliers[unit];
+    } else {
+        return size;
+    }
+};
+
+const processPeriod = (period) => {
+    const result = {
+        num: 1,
+        unit: 'd'
+    };
+
+    // Parse `period`.
+    if (period) {
+        var crackedperiod = {
+            hourly: '1h',
+            daily: '1d',
+            weekly: '1w',
+            monthly: '1m',
+            yearly: '1y'
+        }[period] || period;
+
+        const [num, unit] = /^([1-9][0-9]*)([hdwmy]|ms)$/.exec(crackedperiod).slice(1);
+        
+        result.num = Number(num);
+        result.unit = unit;
+    }
+
+    return result;
+};
+
+module.exports = {
+    processSize,
+    processPeriod
+};