diff --git a/README.md b/README.md index 3809f9d..6b62e73 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ When filename size >= maxSize then: * `compress` \ - defaults to `false` - compress the backup files using gzip (files will have `.gz` extension). * `keepFileExt` \ - defaults to `false` - keep the file original extension. e.g.: `abc.log -> abc.2013-08-30.log`. * `alwaysIncludePattern` \ - defaults to `false` - extend the initial file with the pattern - * `daysToKeep` \ - defaults to `MAX_SAFE_INTEGER` - the number of old files that matches the pattern to keep (excluding the hot file) + * `daysToKeep` \ - defaults to `1` - the number of old files that matches the pattern to keep (excluding the hot file) This returns a `WritableStream`. When the current time, formatted as `pattern`, changes then the current file will be renamed to `filename.formattedDate` where `formattedDate` is the result of processing the date through the pattern, and a new file will begin to be written. Streamroller uses [date-format](http://github.com/nomiddlename/date-format) to format dates, and the `pattern` should use the date-format format. e.g. with a `pattern` of `".yyyy-MM-dd"`, and assuming today is August 29, 2013 then writing to the stream today will just write to `filename`. At midnight (or more precisely, at the next file write after midnight), `filename` will be renamed to `filename.2013-08-29` and a new `filename` will be created. If `options.alwaysIncludePattern` is true, then the initial file will be `filename.2013-08-29` and no renaming will occur at midnight, but a new file will be written to with the name `filename.2013-08-30`. diff --git a/lib/DateRollingFileStream.js b/lib/DateRollingFileStream.js index 9bf52f7..579d31e 100644 --- a/lib/DateRollingFileStream.js +++ b/lib/DateRollingFileStream.js @@ -13,9 +13,15 @@ class DateRollingFileStream extends RollingFileWriteStream { if (!pattern) { pattern = 'yyyy-MM-dd'; } - if (options.daysToKeep) { - options.numToKeep = options.daysToKeep; + if (!options.daysToKeep && options.daysToKeep !== 0) { + options.daysToKeep = 1; + } else if (options.daysToKeep < 0) { + throw new Error(`options.daysToKeep (${options.daysToKeep}) should be >= 0`); + } else if (options.daysToKeep >= Number.MAX_SAFE_INTEGER) { + // to cater for numToKeep (include the hot file) at Number.MAX_SAFE_INTEGER + throw new Error(`options.daysToKeep (${options.daysToKeep}) should be < Number.MAX_SAFE_INTEGER`); } + options.numToKeep = options.daysToKeep + 1; if (pattern.startsWith('.')) { pattern = pattern.substring(1); } diff --git a/lib/RollingFileStream.js b/lib/RollingFileStream.js index ec794c3..cf08bd8 100644 --- a/lib/RollingFileStream.js +++ b/lib/RollingFileStream.js @@ -9,12 +9,17 @@ class RollingFileStream extends RollingFileWriteStream { if (size) { options.maxSize = size; } - if (!backups) { + if (!backups && backups !== 0) { backups = 1; + } else if (backups < 0) { + throw new Error(`backups (${backups}) should be >= 0`); + } else if (backups >= Number.MAX_SAFE_INTEGER) { + // to cater for numToKeep (include the hot file) at Number.MAX_SAFE_INTEGER + throw new Error(`backups (${backups}) should be < Number.MAX_SAFE_INTEGER`); } - options.numToKeep = backups; + options.numToKeep = backups + 1; super(filename, options); - this.backups = this.options.numToKeep; + this.backups = backups; this.size = this.options.maxSize; } diff --git a/lib/RollingFileWriteStream.js b/lib/RollingFileWriteStream.js index c672464..ccaebc2 100644 --- a/lib/RollingFileWriteStream.js +++ b/lib/RollingFileWriteStream.js @@ -250,7 +250,7 @@ class RollingFileWriteStream extends Writable { debug("_clean: existing files are: ", existingFileDetails); if (this._tooManyFiles(existingFileDetails.length)) { const fileNamesToRemove = existingFileDetails - .slice(0, existingFileDetails.length - this.options.numToKeep - 1) + .slice(0, existingFileDetails.length - this.options.numToKeep) .map(f => path.format({ dir: this.fileObject.dir, base: f.filename })); await deleteFiles(fileNamesToRemove); } diff --git a/test/DateRollingFileStream-test.js b/test/DateRollingFileStream-test.js index 8d98e6b..ff4a269 100644 --- a/test/DateRollingFileStream-test.js +++ b/test/DateRollingFileStream-test.js @@ -460,6 +460,28 @@ describe("DateRollingFileStream", function() { }); }); + describe("with invalid number of daysToKeep", () => { + it("should complain about negative daysToKeep", () => { + const daysToKeep = -1; + (() => { + new DateRollingFileStream( + path.join(__dirname, "daysToKeep.log"), + { daysToKeep: daysToKeep } + ); + }).should.throw(`options.daysToKeep (${daysToKeep}) should be >= 0`); + }); + + it("should complain about daysToKeep >= Number.MAX_SAFE_INTEGER", () => { + const daysToKeep = Number.MAX_SAFE_INTEGER; + (() => { + new DateRollingFileStream( + path.join(__dirname, "daysToKeep.log"), + { daysToKeep: daysToKeep } + ); + }).should.throw(`options.daysToKeep (${daysToKeep}) should be < Number.MAX_SAFE_INTEGER`); + }); + }); + describe("with daysToKeep option", function() { let stream; var daysToKeep = 4; @@ -539,7 +561,7 @@ describe("DateRollingFileStream", function() { stream.write("New file message\n", "utf8", done); }); - it("should be 4 files left from original 3", async function() { + it("should be 5 files left from original 11", async function() { const files = await fs.readdir(__dirname); var logFiles = files.filter( file => file.indexOf("compressedDaysToKeep.log") > -1 diff --git a/test/RollingFileStream-test.js b/test/RollingFileStream-test.js index c08f4dd..e45e124 100644 --- a/test/RollingFileStream-test.js +++ b/test/RollingFileStream-test.js @@ -119,6 +119,30 @@ describe("RollingFileStream", function() { }); }); + describe("with invalid number of backups", () => { + it("should complain about negative backups", () => { + const backups = -1; + (() => { + new RollingFileStream( + path.join(__dirname, "test-rolling-file-stream"), + 1024, + backups + ); + }).should.throw(`backups (${backups}) should be >= 0`); + }); + + it("should complain about backups >= Number.MAX_SAFE_INTEGER", () => { + const backups = Number.MAX_SAFE_INTEGER; + (() => { + new RollingFileStream( + path.join(__dirname, "test-rolling-file-stream"), + 1024, + backups + ); + }).should.throw(`backups (${backups}) should be < Number.MAX_SAFE_INTEGER`); + }); + }); + describe("writing less than the file size", function() { before(async function() { await remove("test-rolling-file-stream-write-less"); diff --git a/test/RollingFileWriteStream-test.js b/test/RollingFileWriteStream-test.js index 2e74b06..d1dcdb1 100644 --- a/test/RollingFileWriteStream-test.js +++ b/test/RollingFileWriteStream-test.js @@ -472,7 +472,7 @@ describe("RollingFileWriteStream", () => { }); }); - describe("with 5 maxSize and 3 files limit", () => { + describe("with 5 maxSize and 3 backups limit", () => { const fileObj = generateTestFile(); let s; @@ -480,7 +480,7 @@ describe("RollingFileWriteStream", () => { fakeNow = new Date(2012, 8, 12, 10, 37, 11); s = new RollingFileWriteStream(fileObj.path, { maxSize: 5, - numToKeep: 3 + numToKeep: 4 }); const flows = Array.from(Array(38).keys()).map(i => () => { fakeNow = new Date(2012, 8, 12 + parseInt(i / 5), 10, 37, 11); @@ -543,7 +543,7 @@ describe("RollingFileWriteStream", () => { }); }); - describe("with 5 maxSize and 3 files limit, rotating daily", () => { + describe("with 5 maxSize and 3 backups limit, rotating daily", () => { const fileObj = generateTestFile(); let s; @@ -552,7 +552,7 @@ describe("RollingFileWriteStream", () => { s = new RollingFileWriteStream(fileObj.path, { maxSize: 5, pattern: "yyyy-MM-dd", - numToKeep: 3 + numToKeep: 4 }); const flows = Array.from(Array(38).keys()).map(i => () => { fakeNow = new Date(2012, 8, 12 + parseInt(i / 10), 10, 37, 11);