From e66d805672eb4ff21232c3f3bf36a111dfb4a4fc Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Fri, 14 Apr 2017 14:07:00 -0400 Subject: [PATCH] feat(child-writer): log non-robot messages for debugging purposes (#1295) Currently, if the child writer receives a message from the writer process that is not a valid robot object, then it will throw an error. Now, we check if the message is a robot message, and if so, we try to parse it (throwing errors if its indeed a malformed/incomplete robot message), however we log it if not. The main motivation behind this feature is that it will allows us to print debugging information on the mountutils module, and have it redirected to DevTools. Change-Type: minor Signed-off-by: Juan Cruz Viotti --- lib/child-writer/index.js | 34 ++++++++++++-- lib/shared/robot/index.js | 28 +++++++++++ tests/shared/robot.spec.js | 95 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) diff --git a/lib/child-writer/index.js b/lib/child-writer/index.js index edbc84c8..a2402903 100644 --- a/lib/child-writer/index.js +++ b/lib/child-writer/index.js @@ -126,8 +126,28 @@ exports.write = (image, drive, options) => { * })); */ const bridgeRobotMessage = (message) => { + const ROBOT_COMMAND_LOG = 'log'; + + const parsedMessage = _.attempt(() => { + if (robot.isMessage(message)) { + return robot.parseMessage(message); + } + + // Don't be so strict. If a message doesn't look like + // a robot message, then make the child writer log it + // for debugging purposes. + return robot.parseMessage(robot.buildMessage(ROBOT_COMMAND_LOG, { + message + })); + + }); + + if (_.isError(parsedMessage)) { + emitError(parsedMessage); + return; + } + try { - const parsedMessage = robot.parseMessage(message); // These are lighweight accessor methods for // the properties of the parsed message @@ -139,8 +159,16 @@ exports.write = (image, drive, options) => { // to provide better encapsulation. if (messageCommand === 'error') { emitError(robot.recomposeErrorMessage(parsedMessage)); - } else if (messageCommand === 'log') { - console.log(messageData); + } else if (messageCommand === ROBOT_COMMAND_LOG) { + + // If the message data is an object and it contains a + // message string then log the message string only. + if (_.isPlainObject(messageData) && _.isString(messageData.message)) { + console.log(messageData.message); + } else { + console.log(messageData); + } + } else { emitter.emit(messageCommand, messageData); } diff --git a/lib/shared/robot/index.js b/lib/shared/robot/index.js index 9b62f1f4..aa387dbe 100644 --- a/lib/shared/robot/index.js +++ b/lib/shared/robot/index.js @@ -65,6 +65,34 @@ exports.buildMessage = (title, data = {}) => { }); }; +/** + * @summary Check whether a string is a robot message + * @function + * @public + * + * @description + * Note that this function doesn't check if the robot message + * is valid, but just that it is a robot message that we should + * attempt to parse. + * + * @param {String} string - string + * @returns {Boolean} whether the string is a robot message + * + * @example + * if (robot.isMessage(robot.buildMessage('foo', { + * message: 'bar' + * }))) { + * console.log('This is a robot message'); + * } + */ +exports.isMessage = (string) => { + try { + return _.isPlainObject(JSON.parse(string)); + } catch (error) { + return false; + } +}; + /** * @summary Parse a machine-parseable message * @function diff --git a/tests/shared/robot.spec.js b/tests/shared/robot.spec.js index b90ab3c5..8436a1a0 100644 --- a/tests/shared/robot.spec.js +++ b/tests/shared/robot.spec.js @@ -91,6 +91,101 @@ describe('Shared: Robot', function() { }); + describe('.isMessage()', function() { + + it('should return true if message is an empty object', function() { + m.chai.expect(robot.isMessage('{}')).to.be.true; + }); + + it('should return true if message is an object', function() { + m.chai.expect(robot.isMessage('{"command":"foo"}')).to.be.true; + }); + + it('should return false if message is an invalid object', function() { + m.chai.expect(robot.isMessage('{"command":\\foo"}')).to.be.false; + }); + + it('should return false if message is an unquoted string', function() { + m.chai.expect(robot.isMessage('foo')).to.be.false; + }); + + it('should return false if message is an quoted string', function() { + m.chai.expect(robot.isMessage('"foo"')).to.be.false; + }); + + it('should return false if message is an empty string', function() { + m.chai.expect(robot.isMessage('')).to.be.false; + }); + + it('should return false if message is undefined', function() { + m.chai.expect(robot.isMessage(undefined)).to.be.false; + }); + + it('should return false if message is null', function() { + m.chai.expect(robot.isMessage(null)).to.be.false; + }); + + it('should return false if message is a positive integer string', function() { + m.chai.expect(robot.isMessage('5')).to.be.false; + }); + + it('should return false if message is a negative integer string', function() { + m.chai.expect(robot.isMessage('-3')).to.be.false; + }); + + it('should return false if message is a zero string', function() { + m.chai.expect(robot.isMessage('0')).to.be.false; + }); + + it('should return false if message is a positive float string', function() { + m.chai.expect(robot.isMessage('5.3')).to.be.false; + }); + + it('should return false if message is a negative float string', function() { + m.chai.expect(robot.isMessage('-2.1')).to.be.false; + }); + + it('should return false if message is a positive integer', function() { + m.chai.expect(robot.isMessage(5)).to.be.false; + }); + + it('should return false if message is a negative integer', function() { + m.chai.expect(robot.isMessage(-3)).to.be.false; + }); + + it('should return false if message is zero', function() { + m.chai.expect(robot.isMessage(0)).to.be.false; + }); + + it('should return false if message is a positive float', function() { + m.chai.expect(robot.isMessage(5.3)).to.be.false; + }); + + it('should return false if message is a negative float', function() { + m.chai.expect(robot.isMessage(-2.1)).to.be.false; + }); + + it('should return false if message is an array', function() { + m.chai.expect(robot.isMessage([ 'foo' ])).to.be.false; + }); + + it('should return false if message is an array string', function() { + m.chai.expect(robot.isMessage('["foo"]')).to.be.false; + }); + + it('should return true for a message built with .buildMessage()', function() { + m.chai.expect(robot.isMessage(robot.buildMessage('foo', { + message: 'bar' + }))).to.be.true; + }); + + it('should return true for a message built with .buildErrorMessage()', function() { + const error = new Error('foo'); + m.chai.expect(robot.isMessage(robot.buildErrorMessage(error))).to.be.true; + }); + + }); + describe('.buildErrorMessage()', function() { it('should build a message from a simple error', function() {