fix(exit): made exit listeners responsibility of caller

Kurea · Mar 19, 2017 · 2e03528 · 2e03528
1 parent c4a6efa
commit 2e03528
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 169 deletions.
diff --git a/lib/appenders/dateFile.js b/lib/appenders/dateFile.js
@@ -4,12 +4,6 @@ const streams = require('streamroller');
 const os = require('os');
 
 const eol = os.EOL || '\n';
-const appenders = [];
-
-// close open files on process exit.
-process.on('exit', () => {
-  appenders.forEach((a) => { a.shutdown(); });
-});
 
 /**
  * File appender that rolls files according to a date pattern.
@@ -42,8 +36,6 @@ function appender(
     });
   };
 
-  appenders.push(app);
-
   return app;
 }
 
@@ -67,5 +59,4 @@ function configure(config, layouts) {
   );
 }
 
-module.exports.appender = appender;
 module.exports.configure = configure;
diff --git a/lib/appenders/file.js b/lib/appenders/file.js
@@ -6,23 +6,6 @@ const streams = require('streamroller');
 const os = require('os');
 
 const eol = os.EOL || '\n';
-const appenders = [];
-
-// close open files on process exit.
-process.on('exit', () => {
-  debug('Exit handler called.');
-  appenders.forEach((a) => { a.shutdown(); });
-});
-
-// On SIGHUP, close and reopen all files. This allows this appender to work with
-// logrotate. Note that if you are using logrotate, you should not set
-// `logSize`.
-process.on('SIGHUP', () => {
-  debug('SIGHUP handler called.');
-  appenders.forEach((a) => {
-    a.reopen();
-  });
-});
 
 function openTheStream(file, fileSize, numFiles, options) {
   const stream = new streams.RollingFileStream(
@@ -81,8 +64,14 @@ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset
     });
   };
 
-  // push file to the stack of open handlers
-  appenders.push(app);
+  // On SIGHUP, close and reopen all files. This allows this appender to work with
+  // logrotate. Note that if you are using logrotate, you should not set
+  // `logSize`.
+  process.on('SIGHUP', () => {
+    debug('SIGHUP handler called.');
+    app.reopen();
+  });
+
   return app;
 }
 
@@ -102,5 +91,4 @@ function configure(config, layouts) {
   );
 }
 
-module.exports.appender = fileAppender;
 module.exports.configure = configure;
diff --git a/test/tap/dateFileAppender-test.js b/test/tap/dateFileAppender-test.js
@@ -3,7 +3,6 @@
 const test = require('tap').test;
 const path = require('path');
 const fs = require('fs');
-const sandbox = require('sandboxed-module');
 const log4js = require('../../lib/log4js');
 const EOL = require('os').EOL || '\n';
 
@@ -16,73 +15,6 @@ function removeFile(filename) {
 }
 
 test('../../lib/appenders/dateFile', (batch) => {
-  batch.test('adding multiple dateFileAppenders', (t) => {
-    const listenersCount = process.listeners('exit').length;
-
-    log4js.configure({
-      appenders: {
-        date0: { type: 'dateFile', filename: 'datefa-default-test0.log' },
-        date1: { type: 'dateFile', filename: 'datefa-default-test1.log' },
-        date2: { type: 'dateFile', filename: 'datefa-default-test2.log' },
-        date3: { type: 'dateFile', filename: 'datefa-default-test3.log' },
-        date4: { type: 'dateFile', filename: 'datefa-default-test4.log' }
-      },
-      categories: { default: { appenders: ['date0', 'date1', 'date2', 'date3', 'date4'], level: 'debug' } }
-    });
-
-    t.teardown(() => {
-      removeFile('datefa-default-test0.log');
-      removeFile('datefa-default-test1.log');
-      removeFile('datefa-default-test2.log');
-      removeFile('datefa-default-test3.log');
-      removeFile('datefa-default-test4.log');
-    });
-
-    t.equal(process.listeners('exit').length, listenersCount + 1, 'should only add one exit listener');
-    t.end();
-  });
-
-  batch.test('exit listener', (t) => {
-    let exitListener;
-    const openedFiles = [];
-
-    const dateFileAppender = sandbox.require(
-      '../../lib/appenders/dateFile',
-      {
-        globals: {
-          process: {
-            on: function (evt, listener) {
-              exitListener = listener;
-            }
-          }
-        },
-        requires: {
-          streamroller: {
-            DateRollingFileStream: function (filename) {
-              openedFiles.push(filename);
-
-              this.end = function () {
-                openedFiles.shift();
-              };
-
-              this.write = function (data, encoding, cb) {
-                return cb();
-              };
-            }
-          }
-        }
-      }
-    );
-
-    for (let i = 0; i < 5; i += 1) {
-      dateFileAppender.configure({ filename: `test${i}` }, { basicLayout: function () {} });
-    }
-    t.equal(openedFiles.length, 5);
-    exitListener();
-    t.equal(openedFiles.length, 0, 'should close all opened files');
-    t.end();
-  });
-
   batch.test('with default settings', (t) => {
     const testFile = path.join(__dirname, 'date-appender-default.log');
     log4js.configure({

diff --git a/test/tap/fileAppender-test.js b/test/tap/fileAppender-test.js
@@ -17,78 +17,6 @@ function removeFile(filename) {
 }
 
 test('log4js fileAppender', (batch) => {
-  batch.test('adding multiple fileAppenders', (t) => {
-    const initialCount = process.listeners('exit').length;
-    log4js.configure({
-      appenders: {
-        file0: { type: 'file', filename: 'fa-default-test0.log' },
-        file1: { type: 'file', filename: 'fa-default-test1.log' },
-        file2: { type: 'file', filename: 'fa-default-test2.log' },
-        file3: { type: 'file', filename: 'fa-default-test3.log' },
-        file4: { type: 'file', filename: 'fa-default-test4.log' },
-      },
-      categories: { default: { appenders: ['file0', 'file1', 'file2', 'file3', 'file4'], level: 'debug' } }
-    });
-
-    t.tearDown(() => {
-      removeFile('fa-default-test0.log');
-      removeFile('fa-default-test1.log');
-      removeFile('fa-default-test2.log');
-      removeFile('fa-default-test3.log');
-      removeFile('fa-default-test4.log');
-    });
-
-    t.equal(initialCount + 1, process.listeners('exit').length, 'should not add more than one exit listener');
-    t.end();
-  });
-
-  batch.test('exit listener', (t) => {
-    let exitListener;
-    const openedFiles = [];
-
-    const fileAppender = sandbox.require(
-      '../../lib/appenders/file',
-      {
-        globals: {
-          process: {
-            on: function (evt, listener) {
-              if (evt === 'exit') {
-                exitListener = listener;
-              }
-            }
-          }
-        },
-        singleOnly: true,
-        requires: {
-          streamroller: {
-            RollingFileStream: function (filename) {
-              openedFiles.push(filename);
-
-              this.end = function () {
-                openedFiles.shift();
-              };
-
-              this.write = function (data, encoding, cb) {
-                return cb();
-              };
-
-              this.on = function () {
-              };
-            }
-          }
-        }
-      }
-    );
-
-    for (let i = 0; i < 5; i += 1) {
-      fileAppender.configure({ filename: `test${i}` }, { basicLayout: function () {} });
-    }
-    t.equal(openedFiles.length, 5);
-    exitListener();
-    t.equal(openedFiles.length, 0, 'should close all open files');
-    t.end();
-  });
-
   batch.test('with default fileAppender settings', (t) => {
     const testFile = path.join(__dirname, 'fa-default-test.log');
     const logger = log4js.getLogger('default-settings');

diff --git a/v2-changes.md b/v2-changes.md
@@ -0,0 +1,11 @@
+CHANGES
+=======
+
+- no exit listeners defined for appenders by default. users should call log4js.shutdown in their exit listeners.
+- context added to loggers (only logstash uses it so far)
+- logstash split into two appenders (udp and http)
+- no cwd, reload options in config
+- configure only by calling configure, no manual adding of appenders, etc
+- config format changed a lot, now need to define named appenders and at least one category
+- appender format changed, will break any non-core appenders (maybe create adapter?)
+- no replacement of console functions