From 0f41c3b3e1ee7499150dd15158645dc5df47c378 Mon Sep 17 00:00:00 2001 From: Dave Kelsey Date: Tue, 22 May 2018 14:31:55 +0100 Subject: [PATCH] address InvalidRelationship and logging closes #4037 Signed-off-by: Dave Kelsey --- packages/composer-common/lib/log/logger.js | 12 +++++++++- packages/composer-common/test/log/logger.js | 19 +++++++++++++++ .../lib/invalidrelationship.js | 8 +++++-- .../test/invalidrelationship.js | 24 +++++++++---------- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/composer-common/lib/log/logger.js b/packages/composer-common/lib/log/logger.js index 879b8deb49..7a0a67e984 100644 --- a/packages/composer-common/lib/log/logger.js +++ b/packages/composer-common/lib/log/logger.js @@ -175,7 +175,17 @@ class Logger { } // use the local version of padding rather than sprintf etc for speed - _logger.log(logLevel,callbackData+':'+this.padRight(this.str25,this.className)+':'+this.padRight(this.str25,method+'()'),msg, args); + const preamble = callbackData + ':' + this.padRight(this.str25,this.className) + ':' + this.padRight(this.str25,method+'()'); + + try { + _logger.log(logLevel, preamble, msg, args); + } catch(error) { + // an error can be thrown if for example using the winsonInjector logger and an argument is + // an InvalidRelationship where attempts to get object defined properties (which the winstonInjecttor does) + // throws an error. + let safeArgs = args.map(arg => arg.toString()); + _logger.log(logLevel, preamble, msg, safeArgs); + } } } diff --git a/packages/composer-common/test/log/logger.js b/packages/composer-common/test/log/logger.js index a378853b92..9d5a2f8565 100644 --- a/packages/composer-common/test/log/logger.js +++ b/packages/composer-common/test/log/logger.js @@ -122,6 +122,25 @@ describe('Logger', () => { sinon.assert.calledWith(stubLogger.log, 'debug', sinon.match(/ScriptManager.*methodname\(\)/), 'message',['arg1','arg2','arg3']); }); + it('should handle internal logger throwing an error', () => { + let stubLogger= { + log: sinon.stub().onFirstCall().throws(new Error('I do not like the args')) + }; + + let stubArg2 = { + toString: sinon.stub().returns('My To String') + }; + + Logger.setFunctionalLogger(stubLogger); + Logger._envDebug='composer[debug]:*'; + let logger = new Logger('ScriptManager'); + + logger.intlog('debug','methodname','message','arg1',stubArg2,'arg3'); + sinon.assert.calledTwice(stubLogger.log); + sinon.assert.calledWith(stubLogger.log.firstCall, 'debug', sinon.match(/ScriptManager.*methodname\(\)/), 'message',['arg1',stubArg2,'arg3']); + sinon.assert.calledWith(stubLogger.log.secondCall, 'debug', sinon.match(/ScriptManager.*methodname\(\)/), 'message',['arg1','My To String','arg3']); + }); + it('should log to the functional logger, errors', () => { let stubLogger= { log: sinon.stub() diff --git a/packages/composer-runtime/lib/invalidrelationship.js b/packages/composer-runtime/lib/invalidrelationship.js index 7580952b45..8fd64a9309 100644 --- a/packages/composer-runtime/lib/invalidrelationship.js +++ b/packages/composer-runtime/lib/invalidrelationship.js @@ -60,10 +60,14 @@ class InvalidRelationship extends Relationship { configurable: false, enumerable: true, get: () => { - throw error; + const err = new Error(`attempt to get property ${propertyName} on an InvalidRelationship is not allowed. InvalidRelationship created due to ${error.message}`); + LOG.error(err); + throw err; }, set: () => { - throw error; + const err = new Error(`attempt to set property ${propertyName} on an InvalidRelationship is not allowed. InvalidRelationship created due to ${error.message}`); + LOG.error(err); + throw err; } }); } diff --git a/packages/composer-runtime/test/invalidrelationship.js b/packages/composer-runtime/test/invalidrelationship.js index 1751232835..5ee13ab379 100644 --- a/packages/composer-runtime/test/invalidrelationship.js +++ b/packages/composer-runtime/test/invalidrelationship.js @@ -73,73 +73,73 @@ describe('InvalidRelationship', () => { it('should throw attempting to get the primitive value', () => { (() => { relationship.value === 'hello'; - }).should.throw(/such error/); + }).should.throw(/ such error/); }); it('should throw attempting to set the primitive value', () => { (() => { relationship.value = 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to get the primitive array value', () => { (() => { relationship.values[1] === 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to set the primitive array value', () => { (() => { relationship.values[1] = 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to get the concept value', () => { (() => { relationship.concept.value === 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to set the concept value', () => { (() => { relationship.concept.value = 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to get the concept array value', () => { (() => { relationship.concepts[1].value === 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to set the concept array value', () => { (() => { relationship.concepts[1].value = 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to get the relationship value', () => { (() => { relationship.owner.value === 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to set the relationship value', () => { (() => { relationship.owner.value = 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to get the relationship array value', () => { (() => { relationship.owners[1].value === 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); it('should throw attempting to set the relationship array value', () => { (() => { relationship.owners[1].value = 'hello'; - }).should.throw(/such error/); + }).should.throw(/.*is not allowed.*such error/); }); });