8000 fix(exit): made exit listeners responsibility of caller · commontime/log4js-node@2e03528 · GitHub
[go: up one dir, main page]

Skip to content

Commit 2e03528

Browse files
author
Gareth Jones
committed
fix(exit): made exit listeners responsibility of caller
1 parent c4a6efa commit 2e03528

File tree

5 files changed

+19
-169
lines changed

5 files changed

+19
-169
lines changed

lib/appenders/dateFile.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,6 @@ const streams = require('streamroller');
44
const os = require('os');
55

66
const eol = os.EOL || '\n';
7-
const appenders = [];
8-
9-
// close open files on process exit.
10-
process.on('exit', () => {
11-
appenders.forEach((a) => { a.shutdown(); });
12-
});
137

148
/**
159
* File appender that rolls files according to a date pattern.
@@ -42,8 +36,6 @@ function appender(
4236
});
4337
};
4438

45-
appenders.push(app);
46-
4739
return app;
4840
}
4941

@@ -67,5 +59,4 @@ function configure(config, layouts) {
6759
);
6860
}
6961

70-
module.exports.appender = appender;
7162
module.exports.configure = configure;

lib/appenders/file.js

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,6 @@ const streams = require('streamroller');
66
const os = require('os');
77

88
const eol = os.EOL || '\n';
9-
const appenders = [];
10-
11-
// close open files on process exit.
12-
process.on('exit', () => {
13-
debug('Exit handler called.');
14-
appenders.forEach((a) => { a.shutdown(); });
15-
});
16-
17-
// On SIGHUP, close and reopen all files. This allows this appender to work with
18-
// logrotate. Note that if you are using logrotate, you should not set
19-
// `logSize`.
20-
process.on('SIGHUP', () => {
21-
debug('SIGHUP handler called.');
22-
appenders.forEach((a) => {
23-
a.reopen();
24-
});
25-
});
269

2710
function openTheStream(file, fileSize, numFiles, options) {
2811
const stream = new streams.RollingFileStream(
@@ -81,8 +64,14 @@ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset
8164
});
8265
};
8366

84-
// push file to the stack of open handlers
85-
appenders.push(app);
67+
// On SIGHUP, close and reopen all files. This allows this appender to work with
68+
// logrotate. Note that if you are using logrotate, you should not set
69+
// `logSize`.
70+
process.on('SIGHUP', () => {
71+
debug('SIGHUP handler called.');
72+
app.reopen();
73+
});
74+
8675
return app;
8776
}
8877

@@ -102,5 +91,4 @@ function configure(config, layouts) {
10291
);
10392
}
10493

105-
module.exports.appender = fileAppender;
10694
module.exports.configure = configure;

test/tap/dateFileAppender-test.js

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
const test = require('tap').test;
44
const path = require('path');
55
const fs = require('fs');
6-
const sandbox = require('sandboxed-module');
76
const log4js = require('../../lib/log4js');
87
const EOL = require('os').EOL || '\n';
98

@@ -16,73 +15,6 @@ function removeFile(filename) {
1615
}
1716

1817
test('../../lib/appenders/dateFile', (batch) => {
19-
batch.test('adding multiple dateFileAppenders', (t) => {
20-
const listenersCount = process.listeners('exit').length;
21-
22-
log4js.configure({
23-
appenders: {
24-
date0: { type: 'dateFile', filename: 'datefa-default-test0.log' },
25-
date1: { type: 'dateFile', filename: 'datefa-default-test1.log' },
26-
date2: { type: 'dateFile', filename: 'datefa-default-test2.log' },
27-
date3: { type: 'dateFile', filename: 'datefa-default-test3.log' },
28-
date4: { type: 'dateFile', filename: 'datefa-default-test4.log' }
29-
},
30-
categories: { default: { appenders: ['date0', 'date1', 'date2', 'date3', 'date4'], level: 'debug' } }
31-
});
32-
33-
t.teardown(() => {
34-
removeFile('datefa-default-test0.log');
35-
removeFile('datefa-default-test1.log');
36-
removeFile('datefa-default-test2.log');
37-
removeFile('datefa-default-test3.log');
38-
removeFile('datefa-default-test4.log');
39-
});
40-
41-
t.equal(process.listeners('exit').length, listenersCount + 1, 'should only add one exit listener');
42-
t.end();
43-
});
44-
45-
batch.test('exit listener', (t) => {
46-
let exitListener;
47-
const openedFiles = [];
48-
49-
const dateFileAppender = sandbox.require(
50-
'../../lib/appenders/dateFile',
51-
{
52-
globals: {
53-
process: {
54-
on: function (evt, listener) {
55-
exitListener = listener;
56-
}
57-
}
58-
},
59-
requires: {
60-
streamroller: {
61-
DateRollingFileStream: function (filename) {
62-
openedFiles.push(filename);
63-
64-
this.end = function () {
65-
openedFiles.shift();
66-
};
67-
68-
this.write = function (data, encoding, cb) {
69-
return cb();
70-
};
71-
}
72-
}
73-
}
74-
}
75-
);
76-
77-
for (let i = 0; i < 5; i += 1) {
78-
dateFileAppender.configure({ filename: `test${i}` }, { basicLayout: function () {} });
79-
}
80-
t.equal(openedFiles.length, 5);
81-
exitListener();
82-
t.equal(openedFiles.length, 0, 'should close all opened files');
83-
t.end();
84-
});
85-
8618
batch.test('with default settings', (t) => {
8719
const testFile = path.join(__dirname, 'date-appender-default.log');
8820
log4js.configure({

test/tap/fileAppender-test.js

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -17,78 +17,6 @@ function removeFile(filename) {
1717
}
1818

1919
test('log4js fileAppender', (batch) => {
20-
batch.test('adding multiple fileAppenders', (t) => {
21-
const initialCount = process.listeners('exit').length;
22-
log4js.configure({
23-
appenders: {
24-
file0: { type: 'file', filename: 'fa-default-test0.log' },
25-
file1: { type: 'file', filename: 'fa-default-test1.log' },
26-
file2: { type: 'file', filename: 'fa-default-test2.log' },
27-
file3: { type: 'file', filename: 'fa-default-test3.log' },
28-
file4: { type: 'file', filename: 'fa-default-test4.log' },
29-
},
30-
categories: { default: { appenders: ['file0', 'file1', 'file2', 'file3', 'file4'], level: 'debug' } }
31-
});
32-
33-
t.tearDown(() => {
34-
removeFile('fa-default-test0.log');
35-
removeFile('fa-default-test1.log');
36-
removeFile('fa-default-test2.log');
37-
removeFile('fa-default-test3.log');
38-
removeFile('fa-default-test4.log');
39-
});
40-
41-
t.equal(initialCount + 1, process.listeners('exit').length, 'should not add more than one exit listener');
42-
t.end();
43-
});
44-
45-
batch.test('exit listener', (t) => {
46-
let exitListener;
47-
const openedFiles = [];
48-
49-
const fileAppender = sandbox.require(
50-
'../../lib/appenders/file',
51-
{
52-
globals: {
53-
process: {
54-
on: function (evt, listener) {
55-
if (evt === 'exit') {
56-
exitListener = listener;
57-
}
58-
}
59-
}
60-
},
61-
singleOnly: true,
62-
requires: {
63-
streamroller: {
64-
RollingFileStream: function (filename) {
65-
openedFiles.push(filename);
66-
67-
this.end = function () {
68-
openedFiles.shift();
69-
};
70-
71-
this.write = function (data, encoding, cb) {
72-
return cb();
73-
};
74-
75-
this.on = function () {
76-
};
77-
}
78-
}
79-
}
80-
}
81-
);
82-
83-
for (let i = 0; i < 5; i += 1) {
84-
fileAppender.configure({ filename: `test${i}` }, { basicLayout: function () {} });
85-
}
86-
t.equal(openedFiles.length, 5);
87-
exitListener();
88-
t.equal(openedFiles.length, 0, 'should close all open files');
89-
t.end();
90-
});
91-
9220
batch.test('with default fileAppender settings', (t) => {
9321
const testFile = path.join(__dirname, 'fa-default-test.log');
9422
const logger = log4js.getLogger('default-settings');

v2-changes.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CHANGES
2+
=======
3+
4+
- no exit listeners defined for appenders by default. users should call log4js.shutdown in their exit listeners.
5+
- context added to loggers (only logstash uses it so far)
6+
- logstash split into two appenders (udp and http)
7+
- no cwd, reload options in config
8+
- configure only by calling configure, no manual adding of appenders, etc
9+
- config format changed a lot, now need to define named appenders and at least one category
10+
- appender format changed, will break any non-core appenders (maybe create adapter?)
11+
- no replacement of console functions

0 commit comments

Comments
 (0)
0