Skip to content

Commit ad6b8b3

Browse files
alexk-blackopsalexk-blackops
alexk-blackops
authored and
alexk-blackops
committed
fixes after code review
1 parent 5cc6baf commit ad6b8b3

File tree

8 files changed

+68
-79
lines changed

8 files changed

+68
-79
lines changed

‎config/config.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ module.exports = {
1111
FIVE_MINUTES_DELAY : 300000,
1212
},
1313
MSG: {
14-
QUEUE_CAP : 10000, // queue cap
15-
MIN_BATCH_SIZE : 10, // maximum batch size
16-
MAX_BATCH_SIZE : 100, // maximum batch size
14+
QUEUE_CAP : 10000,
15+
MIN_BATCH_SIZE : 10,
16+
MAX_BATCH_SIZE : 100,
1717
MAX_DUP_ERROR_PER_MINUTE: 5, // Number of instances of a unique error that are allowed to be sent in one minute
1818

1919
},

‎index.js‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
var api = require('./lib/api'), // wrappers for API calls
2-
CONFIG = require('./config/config'), // configuration settings file
3-
error = require('./lib/error'), // error parser module
42
exception = require('./lib/exception'), // exception handler module
5-
helpers = require('./lib/helpers'), // different helpers functions
6-
logger = require('./lib/logger'), // logging methods module
7-
sender = require('./lib/sender'); // wrapper for http/https requests
3+
logger = require('./lib/logger'); // logging methods module
84

95
module.exports = {
106
// start sending logs

‎lib/api.js‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
var os = require('os'),
2-
url = require('url'),
3-
util = require('util'),
42

53
send = require('./sender'),
64
logger = require('./logger'),
@@ -55,9 +53,7 @@ module.exports.methods = {
5553
//start sending logs unless it's not already being sent because of exception
5654

5755
if (!exc.excCaught) {
58-
logger.methods.send();
59-
} else {
60-
console.log('exc caught not sending');
56+
logger.methods.start();
6157
}
6258

6359
},
@@ -82,7 +78,11 @@ module.exports.methods = {
8278
}
8379

8480
},
85-
81+
/* posting logs to API.
82+
messages - array of messages,
83+
cb - callback function
84+
shutdown - optional parameter, indicating if we should retry request attempts when API is down
85+
*/
8686
postLogs: function postLogs(messages, cb, shutdown) {
8787
var delay = 0, // scheduled delay when postLogs call failed
8888
body = {
@@ -103,23 +103,22 @@ module.exports.methods = {
103103

104104
//retry the request if it failed
105105
fail = function (code) {
106-
if (!shutdown) {
107-
var sinceFirstError;
108-
109-
if (code === 401) {
110-
delay = CONFIG.DELAY.FIVE_MINUTES;
111-
lastApiError = new Date().getTime();
112-
} else {
113-
lastApiError = lastApiError || new Date().getTime();
114-
sinceFirstError = new Date().getTime() - lastApiError;
115-
delay = Math.min(Math.max(sinceFirstError, CONFIG.DELAY.ONE_SECOND), CONFIG.DELAY.ONE_MINUTE);
116-
}
117-
118-
setTimeout(function () {
119-
send(opt, cb, fail);
120-
}, delay);
121-
}
106+
var sinceFirstError;
107+
108+
if (code === 401) {
109+
delay = CONFIG.DELAY.FIVE_MINUTES;
110+
lastApiError = new Date().getTime();
111+
} else {
112+
lastApiError = lastApiError || new Date().getTime();
113+
sinceFirstError = new Date().getTime() - lastApiError;
114+
delay = Math.min(Math.max(sinceFirstError, CONFIG.DELAY.ONE_SECOND), CONFIG.DELAY.ONE_MINUTE);
115+
}
116+
117+
setTimeout(function () {
118+
send(opt, cb, fail);
119+
}, delay);
122120
};
123-
send(opt, cb, fail);
121+
122+
send(opt, cb, shutdown ? null : fail);
124123
}
125124
};

‎lib/error.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
var url = require('url'),
2-
util = require('util'),
32
os = require('os'),
43

54
helpers = require('./helpers'),

‎lib/exception.js‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
var qs = require('querystring'),
2-
util = require('util'),
32

43
error = require('./error'),
5-
logger = require('./logger'),
6-
api = require('./api'),
7-
CONFIG = require('../config/config');
4+
logger = require('./logger');
85

96
module.exports = {
107
// flag used to prevent catching the same exception twice (inside and outside of the createServer method)
@@ -27,10 +24,11 @@ module.exports = {
2724
process.on('uncaughtException', function (err) {
2825
if (!self.excCaught) {
2926
self.excCaught = true;
30-
logger.methods.stop(err, req, function () {return true; });
31-
if (exit === true) {
32-
process.exit(1);
33-
}
27+
logger.methods.sendException(err, req, function () {
28+
if (exit === true) {
29+
process.exit(1);
30+
}
31+
});
3432
}
3533
});
3634
}());
@@ -53,7 +51,7 @@ module.exports = {
5351

5452
if (!self.excCaught) {
5553
self.excCaught = true;
56-
logger.methods.stop(err, req, cb);
54+
logger.methods.sendException(err, req, cb);
5755
}
5856

5957
};

‎lib/helpers.js‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
var stackTrace = require('stack-trace');
2-
util = require('util');
1+
var stackTrace = require('stack-trace'),
2+
util = require('util'),
3+
4+
CONFIG = require('../config/config');
5+
36
/*
47
*** Function for parsing meta objects attached to the message
58
*/
69

7-
810
module.exports.parseMeta = function parseMeta(meta, err) {
911
var result,
1012
ex,
@@ -32,9 +34,10 @@ module.exports.parseMeta = function parseMeta(meta, err) {
3234

3335
};
3436

35-
/*
37+
/*
3638
*** Function for extracting cookies from request object ***
3739
*/
40+
3841
module.exports.getCookies = function getCookies(req) {
3942
var key,
4043
keys,
@@ -59,7 +62,7 @@ module.exports.getCookies = function getCookies(req) {
5962
return result;
6063
};
6164

62-
/*
65+
/*
6366
*** Function for extracting post data from request object ***
6467
*/
6568

@@ -71,7 +74,7 @@ module.exports.getPostData = function getPostData(req) {
7174
return result;
7275
};
7376

74-
/*
77+
/*
7578
*** Function for getting the trace of an exception object ***
7679
*/
7780

@@ -93,7 +96,7 @@ module.exports.getTrace = function getTrace(err) {
9396
return result;
9497
};
9598

96-
/*
99+
/*
97100
*** Function for correctly parsing the URL ***
98101
*/
99102

‎lib/logger.js‎

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
var util = require('util'),
2-
api = require('./api'),
1+
var api = require('./api'),
32
CONFIG = require('../config/config'),
43
helpers = require('./helpers'),
54
error = require('./error'),
6-
exc = require('./exception'),
75

86
storage = [],
97

@@ -12,13 +10,14 @@ var util = require('util'),
1210

1311
// hash that contains all the errors and their number logged during the current minute
1412
errorStorage = {},
15-
// flag indicating if messages are being sent right now
16-
flag = false,
13+
sendingLogsInProgress = false,
1714
// delayed sendLogs call
1815
timeout,
1916

2017

21-
// handler for sending logs
18+
/* handler for sending logs, accepts callback function and boolean variable
19+
indicating if we should run process.exit() after the callback
20+
*/
2221
sendLogs = function (cb, shutdown) {
2322
var length = storage.length, // number of the messages in the iteration
2423
chunk = Math.min(length, CONFIG.MSG.MAX_BATCH_SIZE),
@@ -32,19 +31,17 @@ var util = require('util'),
3231
length -= chunk;
3332
api.lastApiError = 0; // reset the last HTTP error
3433

35-
console.log(chunk, ' sent');
36-
3734
if (length) {
3835
chunk = Math.min(length, CONFIG.MSG.MAX_BATCH_SIZE);
3936
data = storage.slice(0, chunk);
40-
api.methods.postLogs(data, success);
37+
api.methods.postLogs(data, success, shutdown);
4138
} else {
4239
// full batch is sent, set timeout for the next request
4340
if (cb) {
44-
// if exception is caught in Express run next middleware
41+
// if exception is caught run next middleware or check if we need to shutdown the server
4542
cb();
4643
} else {
47-
flag = false;
44+
sendingLogsInProgress = false;
4845
timeout = setTimeout(sendLogs, delay);
4946
}
5047
}
@@ -59,11 +56,11 @@ var util = require('util'),
5956
if (data.length && CONFIG.APIKEY) {
6057
// queue has messages and the app is authorized - set the flag, post data
6158
api.methods.postLogs(data, success, shutdown);
62-
flag = true;
59+
sendingLogsInProgress = true;
6360
} else {
6461
// queue is empty or app is not authorized, set timeout for the next request, switch the flag
6562
timeout = setTimeout(sendLogs, delay);
66-
flag = false;
63+
sendingLogsInProgress = false;
6764
}
6865
},
6966
/**
@@ -73,7 +70,7 @@ var util = require('util'),
7370
checkErrorLimitMessage = function checkErrorLimitMessage(ex) {
7471
var d = new Date(),
7572
min = d.getFullYear().toString() + d.getMonth().toString() + d.getDate().toString()
76-
+ d.getHours().toString() + d.getMinutes().toString();
73+
+ d.getHours().toString() + d.getMinutes().toString(),
7774
key = ex.Error.Message + ex.Error.ErrorType + ex.Error.SourceMethod;
7875

7976
if (!ex) {
@@ -101,7 +98,7 @@ module.exports.methods = {
10198
push : function push(level, msg, meta, req) {
10299
var err = new Error(),
103100
getStack = error.getStackTraceItem(err),
104-
rec = {
101+
messageObject = {
105102
Msg: msg,
106103
Level: level.toUpperCase(),
107104
EpochMs: Date.now(),
@@ -115,42 +112,40 @@ module.exports.methods = {
115112
if (level.toLowerCase() === 'error') {
116113
data = helpers.parseMeta(meta, true);
117114
if (data.ex) {
118-
rec.Ex = error.formatEx(data.ex, req);
115+
messageObject.Ex = error.formatEx(data.ex, req);
119116
}
120117
} else {
121118
data = helpers.parseMeta(meta);
122119
}
123120

124121
if (data.result !== '{}') {
125-
rec.Data = data.result;
122+
messageObject.Data = data.result;
126123
}
127124
}
128125

129-
if (level.toLowerCase() === 'error' && !rec.Ex) {
130-
rec.Ex = error.formatEx(err, null, msg);
126+
if (level.toLowerCase() === 'error' && !messageObject.Ex) {
127+
messageObject.Ex = error.formatEx(err, null, msg);
131128
}
132129

133-
if ((level.toLowerCase() === 'error' && checkErrorLimitMessage(rec.Ex)) || level.toLowerCase() !== 'error') {
134-
storage.push(rec);
135-
console.log('logged ', msg);
130+
if ((level.toLowerCase() === 'error' && checkErrorLimitMessage(messageObject.Ex)) || level.toLowerCase() !== 'error') {
131+
storage.push(messageObject);
136132
// remove the earliest message from the queue if message cap is exceeded
137133
if (storage.length === CONFIG.MSG.QUEUE_CAP) {
138134
storage.shift();
139135
}
140-
}
136+
}
141137
},
142138

143-
/* check the queue after IdentifyApp call is done
139+
/* start sending logs after IdentifyApp call is done
144140
*/
145-
send: function send() {
141+
start: function start() {
146142
timeout = setTimeout(sendLogs, delay);
147143
},
148-
// push exception to the queue and send all the data
149-
stop: function stop(err, req, cb) {
150-
console.log('stopped');
144+
// reset current delay schedule, push exception to the queue, send data and run the callback
145+
sendException: function sendException(err, req, cb) {
151146
var check = function check() {
152147

153-
if (flag) {
148+
if (sendingLogsInProgress) {
154149
setTimeout(check, 500);
155150
} else {
156151
sendLogs(cb, true);

‎lib/sender.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
Low level function for sending http/https requests
66
*/
77
var http = require('http'),
8-
util = require('util'),
98

109
request = require('request'),
1110

@@ -17,7 +16,7 @@ module.exports = function sender(options, cb, fail) {
1716
if (!error) {
1817
if (response.statusCode === 200) {
1918
if (cb) {
20-
cb(body);
19+
cb(body);
2120
}
2221
} else {
2322
if (fail) {

0 commit comments

Comments
 (0)