diff --git a/lib/appenders/index.js b/lib/appenders/index.js index ab78fcc..f6a3c5d 100644 --- a/lib/appenders/index.js +++ b/lib/appenders/index.js @@ -40,6 +40,22 @@ const loadAppenderModule = (type, config) => coreAppenders.get(type) || (require.main && tryLoading(path.join(path.dirname(require.main.filename), type), config)) || tryLoading(path.join(process.cwd(), type), config); +const appendersLoading = new Set(); + +const getAppender = (name, config) => { + if (appenders.has(name)) return appenders.get(name); + if (!config.appenders[name]) return false; + if (appendersLoading.has(name)) throw new Error(`Dependency loop detected for appender ${name}.`); + appendersLoading.add(name); + + debug(`Creating appender ${name}`); + // eslint-disable-next-line no-use-before-define + const appender = createAppender(name, config); + appendersLoading.delete(name); + appenders.set(name, appender); + return appender; +}; + const createAppender = (name, config) => { const appenderConfig = config.appenders[name]; const appenderModule = appenderConfig.type.configure @@ -63,7 +79,7 @@ const createAppender = (name, config) => { return appenderModule.configure( adapters.modifyConfig(appenderConfig), layouts, - appender => appenders.get(appender), + appender => getAppender(appender, config), levels ); }, () => {}); @@ -71,10 +87,9 @@ const createAppender = (name, config) => { const setup = (config) => { appenders.clear(); - + appendersLoading.clear(); Object.keys(config.appenders).forEach((name) => { - debug(`Creating appender ${name}`); - appenders.set(name, createAppender(name, config)); + getAppender(name, config); }); }; diff --git a/test/tap/appender-dependencies-test.js b/test/tap/appender-dependencies-test.js new file mode 100644 index 0000000..0bd06f8 --- /dev/null +++ b/test/tap/appender-dependencies-test.js @@ -0,0 +1,114 @@ +const { test } = require("tap"); + +const categories = { + default: { appenders: ["filtered"], level: "debug" } +} + +let log4js; +let recording; + +test("log4js appender dependencies", batch => { + batch.beforeEach(done => { + log4js = require("../../lib/log4js"); + recording = require("../../lib/appenders/recording"); + done(); + }); + batch.afterEach(done => { + recording.erase(); + done(); + }); + batch.test("in order", t => { + const config = { + categories, + appenders: { + recorder: { type: "recording" }, + filtered: { + type: "logLevelFilter", + appender: "recorder", + level: "ERROR" + } + } + }; + t.test('should resolve if defined in dependency order', assert => { + assert.doesNotThrow(() => { + log4js.configure(config); + }, 'this should not trigger an error'); + assert.end(); + }); + const logger = log4js.getLogger("logLevelTest"); + logger.debug("this should not trigger an event"); + logger.error("this should, though"); + + const logEvents = recording.replay(); + t.test( + "should process log events normally", + assert => { + assert.equal(logEvents.length, 1); + assert.equal(logEvents[0].data[0], "this should, though"); + assert.end(); + } + ); + t.end(); + }); + + batch.test("not in order", t => { + const config = { + categories, + appenders: { + filtered: { + type: "logLevelFilter", + appender: "recorder", + level: "ERROR" + }, + recorder: { type: "recording" }, + } + }; + t.test('should resolve if defined out of dependency order', assert => { + assert.doesNotThrow(() => { + log4js.configure(config); + }, 'this should not trigger an error'); + assert.end(); + }); + const logger = log4js.getLogger("logLevelTest"); + logger.debug("this should not trigger an event"); + logger.error("this should, though"); + + const logEvents = recording.replay(); + t.test( + "should process log events normally", + assert => { + assert.equal(logEvents.length, 1); + assert.equal(logEvents[0].data[0], "this should, though"); + assert.end(); + } + ); + t.end(); + }); + + batch.test("with dependency loop", t => { + const config = { + categories, + appenders: { + filtered: { + type: "logLevelFilter", + appender: "filtered2", + level: "ERROR" + }, + filtered2: { + type: "logLevelFilter", + appender: "filtered", + level: "ERROR" + }, + recorder: { type: "recording" }, + } + }; + t.test('should throw an error if if a dependency loop is found', assert => { + assert.throws(() => { + log4js.configure(config); + }, 'Dependency loop detected for appender filtered.'); + assert.end(); + }); + t.end(); + }); + batch.end(); +});