diff --git a/__tests__/Logger-test.js b/__tests__/Logger-test.js index fbaa710e..429a4914 100644 --- a/__tests__/Logger-test.js +++ b/__tests__/Logger-test.js @@ -9,43 +9,77 @@ const Log = new Logger({ }); test('Test Log.info()', () => { - Log.info('Test', true); + Log.info('Test1', false); + expect(mockServerLoggingCallback).toHaveBeenCalledTimes(0); + Log.info('Test2', true); expect(mockServerLoggingCallback).toHaveBeenCalled(); - expect(mockServerLoggingCallback).toHaveBeenCalledWith({ - parameters: {}, - api_setCookie: false, - message: '[info] Test', - }); + expect(mockServerLoggingCallback).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + api_setCookie: false, + logPacket: expect.any(String), + }) + ); + const packet = JSON.parse(mockServerLoggingCallback.mock.calls[0][1].logPacket); + delete packet[0].timestamp; + delete packet[1].timestamp; + expect(packet).toEqual([ + {message: "[info] Test1", parameters: "", }, + {message: "[info] Test2", parameters: "", }, + ]); }); test('Test Log.alert()', () => { - Log.alert('Test', 0, {}, false); + mockServerLoggingCallback.mockClear(); + Log.alert('Test2', {}, false); expect(mockServerLoggingCallback).toHaveBeenCalled(); - expect(mockServerLoggingCallback).toHaveBeenCalledWith({ - parameters: {}, - api_setCookie: false, - message: '[alrt] Test', - }); + expect(mockServerLoggingCallback).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + api_setCookie: false, + logPacket: expect.any(String), + }) + ); + const packet = JSON.parse(mockServerLoggingCallback.mock.calls[0][1].logPacket); + delete packet[0].timestamp; + expect(packet).toEqual([{message: "[alrt] Test2", parameters: {}}]); }); test('Test Log.warn()', () => { - Log.warn('Test', 0); + mockServerLoggingCallback.mockClear(); + Log.warn('Test2'); expect(mockServerLoggingCallback).toHaveBeenCalled(); - expect(mockServerLoggingCallback).toHaveBeenCalledWith({ - parameters: {}, - api_setCookie: false, - message: '[warn] Test', - }); + expect(mockServerLoggingCallback).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + api_setCookie: false, + logPacket: expect.any(String), + }) + ); + const packet = JSON.parse(mockServerLoggingCallback.mock.calls[0][1].logPacket); + delete packet[0].timestamp; + expect(packet).toEqual([{message: "[warn] Test2", parameters: ''}]); }); test('Test Log.hmmm()', () => { - Log.hmmm('Test', 0); + mockServerLoggingCallback.mockClear(); + Log.hmmm('Test'); + Log.info('Test', true); expect(mockServerLoggingCallback).toHaveBeenCalled(); - expect(mockServerLoggingCallback).toHaveBeenCalledWith({ - parameters: {}, - api_setCookie: false, - message: '[hmmm] Test', - }); + expect(mockServerLoggingCallback).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + api_setCookie: false, + logPacket: expect.any(String), + }) + ); + const packet = JSON.parse(mockServerLoggingCallback.mock.calls[0][1].logPacket); + delete packet[0].timestamp; + delete packet[1].timestamp; + expect(packet).toEqual([ + {message: "[hmmm] Test", parameters: ''}, + {message: "[info] Test", parameters: ''} + ]); }); test('Test Log.client()', () => { diff --git a/lib/Log.jsx b/lib/Log.jsx index 511020cf..f549c676 100644 --- a/lib/Log.jsx +++ b/lib/Log.jsx @@ -6,12 +6,14 @@ import Logger from './Logger'; /** * Network interface for logger. * + * @param {Logger} logger * @param {Object} params * @param {Object} params.parameters * @param {String} params.message + * @return {Promise} */ -function serverLoggingCallback(params) { - API(Network('/api.php')).logToServer(params); +function serverLoggingCallback(logger, params) { + return API(Network('/api.php')).logToServer(params); } /** diff --git a/lib/Logger.jsx b/lib/Logger.jsx index 1f181cdd..8387da4d 100644 --- a/lib/Logger.jsx +++ b/lib/Logger.jsx @@ -1,8 +1,8 @@ import _ from 'underscore'; +const MAX_LOG_LINES_BEFORE_FLUSH = 50; export default class Logger { constructor({serverLoggingCallback, isDebug, clientLoggingCallback}) { - this.TEMP_LOG_LINES_TO_KEEP = 50; // An array of log lines that limits itself to a certain number of entries (deleting the oldest) this.logLines = []; this.serverLoggingCallback = serverLoggingCallback; @@ -21,57 +21,53 @@ export default class Logger { /** * Ask the server to write the log message - * - * @param {String} message The message to write - * @param {Number} recentMessages A number of recent messages to append as context - * @param {Object|String} parameters The parameters to send along with the message */ - logToServer(message, recentMessages, parameters = {}) { - // Optionally append recent log lines as context - let msg = message; - if (recentMessages > 0) { - msg += ' | Context: '; - msg += JSON.stringify(this.get(recentMessages)); - } - - // Output the message to the console too. - if (this.isDebug) { - this.client(`${msg} - ${JSON.stringify(parameters)}`); + logToServer() { + // We do not want to call the server with an empty list or if all the lines has onlyFlushWithOthers=true + if (!this.logLines.length || _.all(this.logLines, l => l.onlyFlushWithOthers)) { + return; } // We don't care about log setting web cookies so let's define it as false - const params = {parameters, message, api_setCookie: false}; - this.serverLoggingCallback(params); + const linesToLog = _.map(this.logLines, l => { + delete l.onlyFlushWithOthers; + return l; + }); + this.logLines = []; + const promise = this.serverLoggingCallback(this, {api_setCookie: false, logPacket: JSON.stringify(linesToLog)}); + if (!promise) { + return; + } + promise.then((response) => { + if (response.requestID) { + this.info('Previous log requestID', false, {requestID: response.requestID}, true); + } + }); } /** * Add a message to the list * @param {String} message * @param {Object|String} parameters The parameters associated with the message + * @param {Boolean} forceFlushToServer Should we force flushing all logs to server? + * @param {Boolean} onlyFlushWithOthers A request will never be sent to the server if all loglines have this set to true */ - add(message, parameters) { + add(message, parameters, forceFlushToServer, onlyFlushWithOthers = false) { const length = this.logLines.push({ message, parameters, + onlyFlushWithOthers, timestamp: (new Date()) }); - // If we're over the limit, remove one line. - if (length > this.TEMP_LOG_LINES_TO_KEEP) { - this.logLines.shift(); + if (this.isDebug) { + this.client(`${message} - ${JSON.stringify(parameters)}`); } - } - /** - * Get the last messageCount lines - * @param {Number} messageCount Number of messages to get (optional, - * defaults to 1) - * @return {array} an array of messages (oldest first) - */ - get(messageCount = 1) { - // Don't get more than we have - const count = Math.min(this.TEMP_LOG_LINES_TO_KEEP, messageCount); - return this.logLines.slice(this.logLines.length - count); + // If we're over the limit, flush the logs + if (length > MAX_LOG_LINES_BEFORE_FLUSH || forceFlushToServer) { + this.logToServer() + } } /** @@ -81,25 +77,21 @@ export default class Logger { * @param {String} message The message to log. * @param {Boolean} sendNow if true, the message will be sent right away. * @param {Object|String} parameters The parameters to send along with the message + * @param {Boolean} onlyFlushWithOthers A request will never be sent to the server if all loglines have this set to true */ - info(message, sendNow, parameters) { - if (sendNow) { - const msg = `[info] ${message}`; - this.logToServer(msg, 0, parameters); - } else { - this.add(message, parameters); - } + info(message, sendNow = false, parameters= '', onlyFlushWithOthers = false) { + const msg = `[info] ${message}`; + this.add(msg, parameters, sendNow, onlyFlushWithOthers); } /** * Logs an alert. * * @param {String} message The message to alert. - * @param {Number} recentMessages A number of recent messages to append as context * @param {Object|String} parameters The parameters to send along with the message * @param {Boolean} includeStackTrace Must be disabled for testing */ - alert(message, recentMessages, parameters = {}, includeStackTrace = true) { + alert(message, parameters = {}, includeStackTrace = true) { const msg = `[alrt] ${message}`; const params = parameters; @@ -107,34 +99,29 @@ export default class Logger { params.stack = JSON.stringify(new Error().stack); } - this.logToServer(msg, recentMessages, params); - this.add(msg, params); + this.add(msg, params, true); } /** * Logs a warn. * * @param {String} message The message to warn. - * @param {Number} recentMessages A number of recent messages to append as context * @param {Object|String} parameters The parameters to send along with the message */ - warn(message, recentMessages, parameters) { + warn(message, parameters = '') { const msg = `[warn] ${message}`; - this.logToServer(msg, recentMessages, parameters); - this.add(msg, parameters); + this.add(msg, parameters, true); } /** * Logs a hmmm. * * @param {String} message The message to hmmm. - * @param {Number} recentMessages A number of recent messages to append as context * @param {Object|String} parameters The parameters to send along with the message */ - hmmm(message, recentMessages, parameters) { + hmmm(message, parameters = '') { const msg = `[hmmm] ${message}`; - this.logToServer(msg, recentMessages, parameters); - this.add(msg, parameters); + this.add(msg, parameters, false); } /** @@ -143,8 +130,6 @@ export default class Logger { * @param {String} message The message to log. */ client(message) { - this.add(message); - if (!this.clientLoggingCallback) { return; }