diff --git a/lib/appenders/file.js b/lib/appenders/file.js index aeccddb..2680fc1 100644 --- a/lib/appenders/file.js +++ b/lib/appenders/file.js @@ -13,27 +13,10 @@ function mainSighupHandler() { }); } -function openTheStream(file, fileSize, numFiles, options) { - const stream = new streams.RollingFileStream( - file, - fileSize, - numFiles, - options - ); - stream.on('error', (err) => { - console.error('log4js.fileAppender - Writing to file %s, error happened ', file, err); // eslint-disable-line - }); - stream.on('drain', () => { - process.emit("log4js:pause", false); - }); - return stream; -} - - /** * File Appender writing the logs to a text file. Supports rolling of logs by size. * - * @param file file log messages will be written to + * @param file the file log messages will be written to * @param layout a function that takes a logEvent and returns a string * (defaults to basicLayout). * @param logSize - the maximum size (in bytes) for a log file, @@ -44,6 +27,9 @@ function openTheStream(file, fileSize, numFiles, options) { * @param timezoneOffset - optional timezone offset in minutes (default system local) */ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { + if (typeof file !== "string" || file.length === 0) { + throw new Error(`Invalid filename: ${file}`); + } file = path.normalize(file); numBackups = (!numBackups && numBackups !== 0) ? 5 : numBackups; @@ -56,6 +42,22 @@ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset timezoneOffset, ')' ); + function openTheStream(filePath, fileSize, numFiles, opt) { + const stream = new streams.RollingFileStream( + filePath, + fileSize, + numFiles, + opt + ); + stream.on('error', (err) => { + console.error('log4js.fileAppender - Writing to file %s, error happened ', filePath, err); // eslint-disable-line + }); + stream.on('drain', () => { + process.emit("log4js:pause", false); + }); + return stream; + } + let writer = openTheStream(file, logSize, numBackups, options); const app = function (loggingEvent) { diff --git a/lib/appenders/fileSync.js b/lib/appenders/fileSync.js index b587dbe..995ed78 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -168,22 +168,32 @@ class RollingFileSync { /** * File Appender writing the logs to a text file. Supports rolling of logs by size. * - * @param file file log messages will be written to + * @param file the file log messages will be written to * @param layout a function that takes a logevent and returns a string * (defaults to basicLayout). * @param logSize - the maximum size (in bytes) for a log file, * if not provided then logs won't be rotated. * @param numBackups - the number of log files to keep after logSize * has been reached (default 5) - * @param timezoneOffset - optional timezone offset in minutes - * (default system local) - * @param options - passed as is to fs options + * @param options - options to be passed to the underlying stream + * @param timezoneOffset - optional timezone offset in minutes (default system local) */ -function fileAppender(file, layout, logSize, numBackups, timezoneOffset, options) { - debug('fileSync appender created'); +function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { + if (typeof file !== "string" || file.length === 0) { + throw new Error(`Invalid filename: ${file}`); + } file = path.normalize(file); numBackups = (!numBackups && numBackups !== 0) ? 5 : numBackups; + debug( + 'Creating fileSync appender (', + file, ', ', + logSize, ', ', + numBackups, ', ', + options, ', ', + timezoneOffset, ')' + ); + function openTheStream(filePath, fileSize, numFiles) { let stream; @@ -234,8 +244,8 @@ function configure(config, layouts) { layout, config.maxLogSize, config.backups, - config.timezoneOffset, - options + options, + config.timezoneOffset ); } diff --git a/test/tap/fileAppender-test.js b/test/tap/fileAppender-test.js index 95ee8a4..17f3b4b 100644 --- a/test/tap/fileAppender-test.js +++ b/test/tap/fileAppender-test.js @@ -46,6 +46,27 @@ test("log4js fileAppender", batch => { t.end(); }); + batch.test("should give error if invalid filename", async t => { + const file = ""; + const expectedError = new Error(`Invalid filename: ${file}`); + t.throws( + () => + log4js.configure({ + appenders: { + file: { + type: "file", + filename: file + } + }, + categories: { + default: { appenders: ["file"], level: "debug" } + } + }), + expectedError + ); + t.end(); + }); + batch.test("should flush logs on shutdown", async t => { const testFile = path.join(__dirname, "fa-default-test.log"); await removeFile(testFile); diff --git a/test/tap/fileSyncAppender-test.js b/test/tap/fileSyncAppender-test.js index ff4d60c..c7f0190 100644 --- a/test/tap/fileSyncAppender-test.js +++ b/test/tap/fileSyncAppender-test.js @@ -40,6 +40,27 @@ test("log4js fileSyncAppender", batch => { }); }); + batch.test("should give error if invalid filename", async t => { + const file = ""; + const expectedError = new Error(`Invalid filename: ${file}`); + t.throws( + () => + log4js.configure({ + appenders: { + file: { + type: "fileSync", + filename: file + } + }, + categories: { + default: { appenders: ["file"], level: "debug" } + } + }), + expectedError + ); + t.end(); + }); + batch.test("with a max file size and no backups", t => { const testFile = path.join(__dirname, "/fa-maxFileSize-sync-test.log"); const logger = log4js.getLogger("max-file-size");