From c5e9f5ff1e3c4ae97e047eef4d29f705f07aae0d Mon Sep 17 00:00:00 2001 From: vilyapilya <13.veter@gmail.com> Date: Tue, 18 Feb 2020 13:18:53 -0800 Subject: [PATCH 1/6] make changes --- lib/configs.js | 1 + lib/logger.js | 31 ++++++++++++++++++------ test/logger.js | 64 +++++++++++++++++++++++++++++++------------------- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/lib/configs.js b/lib/configs.js index a89c087..98edd5a 100644 --- a/lib/configs.js +++ b/lib/configs.js @@ -21,4 +21,5 @@ module.exports = { , REQUEST_WITH_CREDENTIALS: false , BACKOFF_PERIOD: 3000 , FAILED_BUF_RETENTION_LIMIT: 10000000 + , RETRY_TIMES: 3 }; diff --git a/lib/logger.js b/lib/logger.js index f65fb33..f8ea54a 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -101,6 +101,12 @@ function Logger(key, options) { this._flushLimit = configs.FLUSH_BYTE_LIMIT; } + if (options.retryTimes && Number.isInteger(options.retryTimes)) { + this._retryTimes = options.retryTimes; + } else { + this._retryTimes = configs.RETRY_TIMES; + } + if (options.flushInterval && Number.isInteger(options.flushInterval)) { this._flushInterval = options.flushInterval; } else { @@ -108,7 +114,7 @@ function Logger(key, options) { } if (options.retryTimeout && Number.isInteger(options.retryTimeout)) { - this._retryTimeout = options._retryTimeout; + this._retryTimeout = options.retryTimeout; } else { this._retryTimeout = configs.BACKOFF_PERIOD; } @@ -131,6 +137,7 @@ function Logger(key, options) { this._buf = []; this._meta = {}; this._isLoggingBackedOff = false; + this._attempts = 0; this.source = { hostname: options.hostname || os.hostname() @@ -273,6 +280,7 @@ Logger.prototype._bufferLog = function(message, callback) { if (!this._isLoggingBackedOff && (this._bufByteLength >= this._flushLimit)) { debug('Buffer size meets (or exceeds) flush limit. Immediately flushing'); + this._flush((err) => { if (err) { debug('Received an error while flushing...' + err); @@ -290,6 +298,7 @@ Logger.prototype._bufferLog = function(message, callback) { if (!this._flusher) { debug('No scheduled flush. Scheduling for %d ms from now.', configs.FLUSH_INTERVAL); + this._flusher = setTimeout(() => { this._flush(callback); }, this._flushInterval); @@ -313,6 +322,7 @@ Logger.prototype._sendRequest = function(config, instance) { .then((response) => { if (response && response.status === 200) { instance._isLoggingBackedOff = false; + instance._attempts = 0; return instance.callback(null, { lines: instance.__dbgLines , httpStatus: response.status @@ -324,8 +334,15 @@ Logger.prototype._sendRequest = function(config, instance) { .catch((err) => { if (err.response) instance._isLoggingBackedOff = true; // Return to buffer - instance._buf = JSON.parse(config.data).ls; - instance._bufByteLength = instance.__bufSizePreserve; + instance._buf = instance._buf.concat(JSON.parse(config.data).ls); + instance._bufByteLength += instance.__bufSizePreserve; + + if(!instance._flusher && instance._attempts < instance._retryTimes) { + instance._attempts += 1; + instance._flusher = setTimeout(() => { + instance._flush(instance.callback) + }, instance._retryTimeout); + } let message = 'An error occured while making the request.'; if (err && err.response) { @@ -339,14 +356,15 @@ Logger.prototype._flush = function(callback) { if (!callback || typeof callback !== 'function') { throw new Error('flush function expects a callback'); } + + clearTimeout(this._flusher); + this._flusher = null; + if (this._buf.length === 0) { debug('Nothing to flush'); return callback(); } - clearTimeout(this._flusher); - this._flusher = null; - const data = stringify({ e: 'ls', ls: this._buf }); this._req.qs.now = Date.now(); @@ -374,7 +392,6 @@ Logger.prototype._flush = function(callback) { this._sendRequest(_config, this); }; - /* * Populate short-hand for each supported Log Level */ diff --git a/test/logger.js b/test/logger.js index 479b958..1aea6ac 100644 --- a/test/logger.js +++ b/test/logger.js @@ -532,37 +532,41 @@ describe('ambient meta', function() { describe('HTTP Exception Handling', function() { let httpExcServer; - let countHits = 0; + let countServertHits = 0; let statusCode = 302; - let edgeCaseFlag = false; + let willEventuallySucceed = false; const port = 1336; + const retryTimeout = 500; const options = testHelper.createOptions({ port: port + , retryTimeout: retryTimeout + }); let whenSuccessConnection = 0; beforeEach(function(done) { httpExcServer = http.createServer(function(req, res) { - if (edgeCaseFlag && countHits >= 1) { + if (willEventuallySucceed && countServertHits >= 1) { statusCode = 200; whenSuccessConnection = Date.now(); } res.writeHead(statusCode, {'Content-Type': 'text/plain'}); res.write('Hello World'); - res.end(() => {++countHits;}); + res.end(() => {++countServertHits;}); }); httpExcServer.on('listening', done); httpExcServer.listen(port); }); afterEach(function(done) { - countHits = 0; + countServertHits = 0; httpExcServer.close(); httpExcServer.on('close', function() { httpExcServer = null; done(); }); + willEventuallySucceed = false; sentMeta = []; body = ''; statusCode = 302; @@ -570,39 +574,51 @@ describe('HTTP Exception Handling', function() { }); const httpExcLogger = Logger.createLogger(testHelper.apikey, options); - it('when fails to connect, it should put the _isLoggingBackedOff flag on', function(done) { + it('when fails to connect, it should retry within retryTimeout period', function(done) { + this.timeout(retryTimeout * 3 + 400); + willEventuallySucceed = true; + let logSentTime = Date.now(); httpExcLogger.debug('The line'); setTimeout(function() { - assert(httpExcLogger._isLoggingBackedOff === true); + assert(whenSuccessConnection - logSentTime >= retryTimeout); + assert(httpExcLogger._buf.length === 0); + done(); + }, retryTimeout * 3 + 200); + }); + it('when fails to connect, it should retry only three times and save the log until the next one comes in', function(done) { + this.timeout(retryTimeout * 3 + 400); + //const failedLogger = Logger.createLogger(testHelper.apikey, options); + let logSentTime = Date.now(); + httpExcLogger.debug('The line'); + + setTimeout(function() { + assert(countServertHits === 3); assert(httpExcLogger._buf.length === 1); done(); - }, configs.FLUSH_INTERVAL + 200); + }, retryTimeout * 3 + 200); }); - it('*!!depends on the previouce test!!* Send the log after the previouse one has failed', function(done) { - this.timeout(3500); - edgeCaseFlag = true; - countHits = 1; + it('*!!depends on the previous test!!* Include the log from the previously faliled flush', function(done) { + this.timeout(retryTimeout + 300); + willEventuallySucceed = true; + countServertHits = 1; const thisSendTime = Date.now(); httpExcLogger.debug('The second line'); + assert(httpExcLogger._buf.length === 2); setTimeout(function() { - assert(whenSuccessConnection - thisSendTime >= configs.BACKOFF_PERIOD); - assert(httpExcLogger._buf.length === 0); - assert(httpExcLogger._isLoggingBackedOff === false); + assert(whenSuccessConnection - thisSendTime >= retryTimeout); done(); - }, configs.BACKOFF_PERIOD + 200); + }, retryTimeout + 200); }); - it('*!!depends on the previouce test!!* Should clear backoff after success', function(done) { + it('*!!depends on the previous test!!* Should clear backoff after success', function(done) { this.timeout(3500); - edgeCaseFlag = true; - countHits = 1; + willEventuallySucceed = true; + countServertHits = 1; const thisSendTime = Date.now(); httpExcLogger.debug('The second line'); setTimeout(function() { - assert(whenSuccessConnection - thisSendTime < configs.BACKOFF_PERIOD); - assert(httpExcLogger._buf.length === 0); - assert(httpExcLogger._isLoggingBackedOff === false); + assert(whenSuccessConnection - thisSendTime < retryTimeout); done(); - }, configs.BACKOFF_PERIOD + 200); + }, retryTimeout + 200); }); it('should not exceed the failed buf retention limit', function(done) { this.timeout(3500); @@ -616,7 +632,7 @@ describe('HTTP Exception Handling', function() { const byteSizeOfBuf = sizeof(tooManyFails._buf[0]) * tooManyFails._buf.length; assert(byteSizeOfBuf <= opts.failedBufRetentionLimit); done(); - }, configs.BACKOFF_PERIOD + 200); + }, retryTimeout + 200); }); it('flushAll should recieve err message', function(done) { const opts = testHelper.createOptions({port: port}); From 275a8ec813a4b446e3c8df94a5b9c574b811bd4a Mon Sep 17 00:00:00 2001 From: vilyapilya <13.veter@gmail.com> Date: Tue, 18 Feb 2020 13:21:25 -0800 Subject: [PATCH 2/6] clean --- lib/logger.js | 2 -- test/logger.js | 1 - 2 files changed, 3 deletions(-) diff --git a/lib/logger.js b/lib/logger.js index f8ea54a..4f7bd54 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -280,7 +280,6 @@ Logger.prototype._bufferLog = function(message, callback) { if (!this._isLoggingBackedOff && (this._bufByteLength >= this._flushLimit)) { debug('Buffer size meets (or exceeds) flush limit. Immediately flushing'); - this._flush((err) => { if (err) { debug('Received an error while flushing...' + err); @@ -298,7 +297,6 @@ Logger.prototype._bufferLog = function(message, callback) { if (!this._flusher) { debug('No scheduled flush. Scheduling for %d ms from now.', configs.FLUSH_INTERVAL); - this._flusher = setTimeout(() => { this._flush(callback); }, this._flushInterval); diff --git a/test/logger.js b/test/logger.js index 1aea6ac..217a50f 100644 --- a/test/logger.js +++ b/test/logger.js @@ -587,7 +587,6 @@ describe('HTTP Exception Handling', function() { }); it('when fails to connect, it should retry only three times and save the log until the next one comes in', function(done) { this.timeout(retryTimeout * 3 + 400); - //const failedLogger = Logger.createLogger(testHelper.apikey, options); let logSentTime = Date.now(); httpExcLogger.debug('The line'); From e22c68ecd1c91f3003b547b4af03d16fbfbe8267 Mon Sep 17 00:00:00 2001 From: vilyapilya <13.veter@gmail.com> Date: Tue, 18 Feb 2020 13:36:41 -0800 Subject: [PATCH 3/6] test the RETRY_TIMES config as well --- lib/logger.js | 1 - test/logger.js | 14 ++++++++------ test/testHelper.js | 2 ++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/logger.js b/lib/logger.js index 4f7bd54..6432b7c 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -334,7 +334,6 @@ Logger.prototype._sendRequest = function(config, instance) { // Return to buffer instance._buf = instance._buf.concat(JSON.parse(config.data).ls); instance._bufByteLength += instance.__bufSizePreserve; - if(!instance._flusher && instance._attempts < instance._retryTimes) { instance._attempts += 1; instance._flusher = setTimeout(() => { diff --git a/test/logger.js b/test/logger.js index 217a50f..53179a8 100644 --- a/test/logger.js +++ b/test/logger.js @@ -538,9 +538,11 @@ describe('HTTP Exception Handling', function() { const port = 1336; const retryTimeout = 500; + const retryTimes = 2 const options = testHelper.createOptions({ port: port - , retryTimeout: retryTimeout + , retryTimeout + , retryTimes }); @@ -585,18 +587,18 @@ describe('HTTP Exception Handling', function() { done(); }, retryTimeout * 3 + 200); }); - it('when fails to connect, it should retry only three times and save the log until the next one comes in', function(done) { - this.timeout(retryTimeout * 3 + 400); + it('when fails to connect, it should retry only options.retryTimes and save the log until the next one comes in', function(done) { + this.timeout(retryTimeout * 4 + 400); let logSentTime = Date.now(); httpExcLogger.debug('The line'); setTimeout(function() { - assert(countServertHits === 3); + assert(countServertHits === retryTimes + 1); assert(httpExcLogger._buf.length === 1); done(); - }, retryTimeout * 3 + 200); + }, retryTimeout * 4); }); - it('*!!depends on the previous test!!* Include the log from the previously faliled flush', function(done) { + it('*!!depends on the previous test!!* Include the log from the previously failed flush', function(done) { this.timeout(retryTimeout + 300); willEventuallySucceed = true; countServertHits = 1; diff --git a/test/testHelper.js b/test/testHelper.js index 482d1be..b1e8ed4 100644 --- a/test/testHelper.js +++ b/test/testHelper.js @@ -103,6 +103,7 @@ module.exports.createOptions = function({ , failedBufRetentionLimit = null , retryTimeout = null , flushInterval = null + , retryTimes = 3 , shimProperties } = {}) { return { @@ -116,6 +117,7 @@ module.exports.createOptions = function({ , failedBufRetentionLimit: failedBufRetentionLimit , retryTimeout: retryTimeout , flushInterval: flushInterval + , retryTimes , shimProperties }; }; From c53b0f48d21177789906f9dd3f64445d4564bfde Mon Sep 17 00:00:00 2001 From: vilyapilya <13.veter@gmail.com> Date: Tue, 18 Feb 2020 13:46:34 -0800 Subject: [PATCH 4/6] lints --- lib/logger.js | 4 ++-- test/logger.js | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/logger.js b/lib/logger.js index 6432b7c..3681bd7 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -334,10 +334,10 @@ Logger.prototype._sendRequest = function(config, instance) { // Return to buffer instance._buf = instance._buf.concat(JSON.parse(config.data).ls); instance._bufByteLength += instance.__bufSizePreserve; - if(!instance._flusher && instance._attempts < instance._retryTimes) { + if (!instance._flusher && instance._attempts < instance._retryTimes) { instance._attempts += 1; instance._flusher = setTimeout(() => { - instance._flush(instance.callback) + instance._flush(instance.callback); }, instance._retryTimeout); } diff --git a/test/logger.js b/test/logger.js index 53179a8..3cc42f1 100644 --- a/test/logger.js +++ b/test/logger.js @@ -538,7 +538,7 @@ describe('HTTP Exception Handling', function() { const port = 1336; const retryTimeout = 500; - const retryTimes = 2 + const retryTimes = 2; const options = testHelper.createOptions({ port: port , retryTimeout @@ -579,7 +579,7 @@ describe('HTTP Exception Handling', function() { it('when fails to connect, it should retry within retryTimeout period', function(done) { this.timeout(retryTimeout * 3 + 400); willEventuallySucceed = true; - let logSentTime = Date.now(); + const logSentTime = Date.now(); httpExcLogger.debug('The line'); setTimeout(function() { assert(whenSuccessConnection - logSentTime >= retryTimeout); @@ -589,7 +589,6 @@ describe('HTTP Exception Handling', function() { }); it('when fails to connect, it should retry only options.retryTimes and save the log until the next one comes in', function(done) { this.timeout(retryTimeout * 4 + 400); - let logSentTime = Date.now(); httpExcLogger.debug('The line'); setTimeout(function() { From ec4f9f197932e66e7f422a9060ab87951cbc4bd9 Mon Sep 17 00:00:00 2001 From: vilyapilya <13.veter@gmail.com> Date: Tue, 18 Feb 2020 13:52:06 -0800 Subject: [PATCH 5/6] check if the attmpets are nullified after success --- test/logger.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/logger.js b/test/logger.js index 3cc42f1..207e4de 100644 --- a/test/logger.js +++ b/test/logger.js @@ -609,7 +609,7 @@ describe('HTTP Exception Handling', function() { done(); }, retryTimeout + 200); }); - it('*!!depends on the previous test!!* Should clear backoff after success', function(done) { + it('*!!depends on the previous test!!* Should clear backoff after success and nullify attempts', function(done) { this.timeout(3500); willEventuallySucceed = true; countServertHits = 1; @@ -617,6 +617,7 @@ describe('HTTP Exception Handling', function() { httpExcLogger.debug('The second line'); setTimeout(function() { assert(whenSuccessConnection - thisSendTime < retryTimeout); + assert(httpExcLogger._attempts === 0) done(); }, retryTimeout + 200); }); From 5a94149acf1ca2423362a812ad7c2d5baeb1eec6 Mon Sep 17 00:00:00 2001 From: vilyapilya <13.veter@gmail.com> Date: Tue, 18 Feb 2020 13:53:02 -0800 Subject: [PATCH 6/6] lints --- test/logger.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/logger.js b/test/logger.js index 207e4de..0a3f9b1 100644 --- a/test/logger.js +++ b/test/logger.js @@ -617,7 +617,7 @@ describe('HTTP Exception Handling', function() { httpExcLogger.debug('The second line'); setTimeout(function() { assert(whenSuccessConnection - thisSendTime < retryTimeout); - assert(httpExcLogger._attempts === 0) + assert(httpExcLogger._attempts === 0); done(); }, retryTimeout + 200); });