Improve Logger class

__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()', () => {

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);
 }
 
 /**

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,60 +77,51 @@ 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;
 
         if (includeStackTrace) {
             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;
         }