From 4d673ee1823d130ae13048bcf0ae3eab1b380d78 Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Fri, 4 Jan 2019 21:47:41 +0000 Subject: [PATCH 1/8] Started work on adding timeout to operation promises --- lib/bindConnectors/index.js | 94 ++++++++++++++++++++++++------------- package.json | 4 +- 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index 230b2c93..c0d77e6b 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -26,24 +26,29 @@ module.exports = function (config, options) { // Dont wait for event loop to be empty when callback is called context.callbackWaitsForEmptyEventLoop = false; + var promiseTimeout = ( + _.isPlainObject(context) && _.isFunction(context.getRemainingTimeInMillis) ? + context.context.getRemainingTimeInMillis() - 10 : + 0 + ); + // The array of events should all be executed in parallel var promises = _.map(events, function (event) { - return when.promise(function (resolve, reject) { + + var messagePromise = when.promise(function (resolve, reject) { // Handle error invalid payload if (!event.header || !event.header.message) { - return resolve( - formatError( - { - headers: {}, - body: { - code: 'invalid_input', - message: 'Invalid message received.' - } - }, - event - ) - ); + return resolve(formatError( + { + headers: {}, + body: { + code: 'invalid_input', + message: 'Invalid message received.' + } + }, + event + )); } @@ -63,18 +68,16 @@ module.exports = function (config, options) { // Handle message not existing if (!_.isFunction(hooks[event.header.message])) { - return resolve( - formatError( - { - headers: {}, - body: { - code: 'not_implemented', - message: 'Could not find a valid message handler for ' + event.header.message - } - }, - event - ) - ); + return resolve(formatError( + { + headers: {}, + body: { + code: 'not_implemented', + message: 'Could not find a valid message handler for ' + event.header.message + } + }, + event + )); } // Got this far? All good, let's run @@ -87,19 +90,44 @@ module.exports = function (config, options) { .done( function (response) { - resolve( - ( _.isError(response.body) ? - formatError(response, event) : - formatMessage(event, response, response.version) - ) - ); - }, function (response) { + resolve(( + _.isError(response.body) ? + formatError(response, event) : + formatMessage(event, response, response.version) + )); + }, + function (response) { resolve(formatError(response, event)); } ); - }); + + if (promiseTimeout) { + + messagePromise + + .timeout(promiseTimeout) + + //Perform this catch only if it is due to timeout + .catch(when.TimeoutError, function () { + // eslint-disable-next-line no-console + console.warn('The promise has not been closed within the time limit.'); + return when.reject(formatError( + { + code: '#fatal_error', + message: 'The operation timed out.', + payload: 'The operation has timed out due to the promise not closing (resolving/rejecting) within the time limit.' + }, + event + )); + }); + + } + + + return messagePromise; + }); // Wait for all of the above to finish, then resolve diff --git a/package.json b/package.json index 91c8f84e..e212e0e3 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@trayio/falafel", - "version": "1.14.1", - "description": "", + "version": "1.15.0", + "description": "A framework for developing and running connectors.", "main": "lib/index.js", "scripts": { "test": "echo \"Error: no test specified\" && exit 1" From 7b1ece6d8c7bb8317078b743a06b7237ffd852eb Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Mon, 7 Jan 2019 21:24:27 +0000 Subject: [PATCH 2/8] Started writing test for bindConnectors --- lib/bindConnectors/index.js | 4 +++ test/bindConnectors/index.js | 57 ++++++++++++++++++++++++++++++++++-- test/falafel.js | 6 ++-- 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index c0d77e6b..25522adb 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -26,6 +26,10 @@ module.exports = function (config, options) { // Dont wait for event loop to be empty when callback is called context.callbackWaitsForEmptyEventLoop = false; + /* + If context.getRemainingTimeInMillis is available, set it's value + minus 10 seconds, else set the timeout to 0 (such that it is falsy) + */ var promiseTimeout = ( _.isPlainObject(context) && _.isFunction(context.getRemainingTimeInMillis) ? context.context.getRemainingTimeInMillis() - 10 : diff --git a/test/bindConnectors/index.js b/test/bindConnectors/index.js index ba5b355f..8401bc4e 100644 --- a/test/bindConnectors/index.js +++ b/test/bindConnectors/index.js @@ -1,10 +1,61 @@ var assert = require('assert'); -var _ = require('lodash'); +global._ = require('lodash'); +global.when = require('when'); var bindConnectors = require('../../lib/bindConnectors'); +/* eslint-disable no-console */ +describe.only('#bindConnectors', function () { -describe.skip('#bindConnectors', function () { + global.falafel = {}; + var config = [], + options = {}; + config.push({ + name: 'test_connector', + globalModel: {}, + globalSchema: {}, + messages: [ + { + name: 'test_op', + schema: { + name: 'test_op' + }, + model: function (params) { + return params; + }, + } + ] -}); \ No newline at end of file + }); + + var boundConnectors = bindConnectors(config, options); + + it('should pass simple function operation run', function (done) { + + boundConnectors( + [ + { + id: 'testID', + header: { + message: 'test_op' + } + } + ], + { + + }, + function (err, resArr) { + if (err) { + assert.fail(err); + } else { + console.log('resArr'); + console.log(resArr); + done(); + } + } + ); + + }); + +}); diff --git a/test/falafel.js b/test/falafel.js index ee994d65..44ff31f4 100644 --- a/test/falafel.js +++ b/test/falafel.js @@ -19,9 +19,9 @@ describe('falafel', function () { directory: __dirname+'/sample' }); - assert(_.isObject(GLOBAL.falafel)); - assert(GLOBAL._); - assert(GLOBAL.when); + assert(_.isObject(global.falafel)); + assert(global._); + assert(global.when); }); it('should create the connectors.json in dev mode', function () { From f2e76b98aa7eb6875cb568b81381fd5ae3b199d9 Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Tue, 8 Jan 2019 22:16:02 +0000 Subject: [PATCH 3/8] Written some tests for bindConnectors --- lib/bindConnectors/index.js | 13 ++- test/bindConnectors/index.js | 184 ++++++++++++++++++++++++++++++++++- 2 files changed, 190 insertions(+), 7 deletions(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index 25522adb..e393caec 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -32,7 +32,7 @@ module.exports = function (config, options) { */ var promiseTimeout = ( _.isPlainObject(context) && _.isFunction(context.getRemainingTimeInMillis) ? - context.context.getRemainingTimeInMillis() - 10 : + context.getRemainingTimeInMillis() - 10000 : 0 ); @@ -109,7 +109,7 @@ module.exports = function (config, options) { if (promiseTimeout) { - messagePromise + return messagePromise .timeout(promiseTimeout) @@ -119,9 +119,12 @@ module.exports = function (config, options) { console.warn('The promise has not been closed within the time limit.'); return when.reject(formatError( { - code: '#fatal_error', - message: 'The operation timed out.', - payload: 'The operation has timed out due to the promise not closing (resolving/rejecting) within the time limit.' + headers: {}, + body: { + code: '#fatal_error', + message: 'The operation timed out.', + payload: 'The operation has timed out due to the promise not closing (resolving/rejecting) within the time limit.' + } }, event )); diff --git a/test/bindConnectors/index.js b/test/bindConnectors/index.js index 8401bc4e..1b4e4df3 100644 --- a/test/bindConnectors/index.js +++ b/test/bindConnectors/index.js @@ -24,6 +24,15 @@ describe.only('#bindConnectors', function () { model: function (params) { return params; }, + }, + { + name: 'test_op_promise', + schema: { + name: 'test_op_promise' + }, + model: function (params) { + return when.resolve(params); + }, } ] @@ -39,6 +48,9 @@ describe.only('#bindConnectors', function () { id: 'testID', header: { message: 'test_op' + }, + body: { + hello: 'world' } } ], @@ -49,8 +61,142 @@ describe.only('#bindConnectors', function () { if (err) { assert.fail(err); } else { - console.log('resArr'); - console.log(resArr); + assert.deepEqual(resArr[0].body, { hello: 'world' }); + done(); + } + } + ); + + }); + + it('should pass simple promise function operation run', function (done) { + + boundConnectors( + [ + { + id: 'testID', + header: { + message: 'test_op_promise' + }, + body: { + hello: 'world' + } + } + ], + { + + }, + function (err, resArr) { + if (err) { + assert.fail(err); + } else { + assert.deepEqual(resArr[0].body, { hello: 'world' }); + done(); + } + } + ); + + }); + + + var timeoutConnectors = bindConnectors( + [ + { + name: 'timeout_connector', + globalModel: {}, + globalSchema: {}, + messages: [ + { + name: 'timeout_op', + schema: { + name: 'timeout_op' + }, + model: function (params) { + return params; + }, + }, + { + name: 'timeout_op_promise', + schema: { + name: 'timeout_op_promise' + }, + model: function (params) { + return when.resolve(params); + }, + }, + { + name: 'long_timeout_op_promise', + schema: { + name: 'timeout_op_promise' + }, + model: function (params) { + return when.promise(function (resolve, reject) { + + setTimeout(function () { + resolve(params); + }, 21000); + + }); + }, + } + ] + + } + ], + options + ); + + it('should pass simple function operation run within timeout limit', function (done) { + + timeoutConnectors( + [ + { + id: 'testID', + header: { + message: 'timeout_op' + }, + body: { + hello: 'world' + } + } + ], + { + + }, + function (err, resArr) { + if (err) { + assert.fail(err); + } else { + assert.deepEqual(resArr[0].body, { hello: 'world' }); + done(); + } + } + ); + + }); + + it('should pass simple function promise operation run within timeout limit', function (done) { + + timeoutConnectors( + [ + { + id: 'testID', + header: { + message: 'timeout_op_promise' + }, + body: { + hello: 'world' + } + } + ], + { + + }, + function (err, resArr) { + if (err) { + assert.fail(err); + } else { + assert.deepEqual(resArr[0].body, { hello: 'world' }); done(); } } @@ -58,4 +204,38 @@ describe.only('#bindConnectors', function () { }); + it('should return timeout error for operation not ending within time limit', function (done) { + + this.timeout(25000); + + timeoutConnectors( + [ + { + id: 'testID', + header: { + message: 'long_timeout_op_promise' + }, + body: { + hello: 'world' + } + } + ], + { + getRemainingTimeInMillis: function () { + return 20000; //20s + } + }, + function (err, resArr) { + if (err) { + assert(err); + } else { + console.log(resArr); + assert.fail(resArr); + } + done(); + } + ); + + }); + }); From 6f5317f77c3b0dc1753fe7b8a7c8013a092f0a9c Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Tue, 8 Jan 2019 22:25:45 +0000 Subject: [PATCH 4/8] Added more conditions to timeout value --- lib/bindConnectors/index.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index e393caec..6253ad23 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -26,15 +26,20 @@ module.exports = function (config, options) { // Dont wait for event loop to be empty when callback is called context.callbackWaitsForEmptyEventLoop = false; + /* If context.getRemainingTimeInMillis is available, set it's value - minus 10 seconds, else set the timeout to 0 (such that it is falsy) + minus 10 seconds, and make sure it is positive; else set the + timeout to 0 (such that it is falsy) */ - var promiseTimeout = ( + var timeRemaining = ( _.isPlainObject(context) && _.isFunction(context.getRemainingTimeInMillis) ? - context.getRemainingTimeInMillis() - 10000 : + context.getRemainingTimeInMillis() : 0 ); + var promiseTimeout = timeRemaining - 10000; + promiseTimeout = ( promiseTimeout < 1 ? 0 : promiseTimeout ); + // The array of events should all be executed in parallel var promises = _.map(events, function (event) { @@ -107,6 +112,7 @@ module.exports = function (config, options) { }); + //If a non-0 value is present, add a timeout condition to the promise if (promiseTimeout) { return messagePromise From 357a3877028d594c9ae72cc3c4bec04f88b33454 Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Wed, 9 Jan 2019 17:01:45 +0000 Subject: [PATCH 5/8] Added more tests and some notes --- lib/bindConnectors/index.js | 8 ++- test/bindConnectors/index.js | 98 ++++++++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index 6253ad23..85a1fb41 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -26,7 +26,7 @@ module.exports = function (config, options) { // Dont wait for event loop to be empty when callback is called context.callbackWaitsForEmptyEventLoop = false; - + /* If context.getRemainingTimeInMillis is available, set it's value minus 10 seconds, and make sure it is positive; else set the @@ -123,13 +123,17 @@ module.exports = function (config, options) { .catch(when.TimeoutError, function () { // eslint-disable-next-line no-console console.warn('The promise has not been closed within the time limit.'); + //NOTE: this reject will force this lambda function to error (not the operation only) return when.reject(formatError( { headers: {}, body: { code: '#fatal_error', message: 'The operation timed out.', - payload: 'The operation has timed out due to the promise not closing (resolving/rejecting) within the time limit.' + payload: { + reason: 'The operation has timed out due to the promise not closing (resolving/rejecting) within the time limit.', + operation: event.header.message + } } }, event diff --git a/test/bindConnectors/index.js b/test/bindConnectors/index.js index 1b4e4df3..33bd1cf9 100644 --- a/test/bindConnectors/index.js +++ b/test/bindConnectors/index.js @@ -33,6 +33,27 @@ describe.only('#bindConnectors', function () { model: function (params) { return when.resolve(params); }, + }, + { + name: 'test_fail_op', + schema: { + name: 'test_fail_op' + }, + model: function (params) { + throw new Error('Throw error.'); + }, + }, + { + name: 'test_fail_op_promise', + schema: { + name: 'test_fail_op_promise' + }, + model: function (params) { + return when.reject({ + code: '#connector_error', + message: 'Reject error' + }); + }, } ] @@ -62,8 +83,8 @@ describe.only('#bindConnectors', function () { assert.fail(err); } else { assert.deepEqual(resArr[0].body, { hello: 'world' }); - done(); } + done(); } ); @@ -91,14 +112,81 @@ describe.only('#bindConnectors', function () { assert.fail(err); } else { assert.deepEqual(resArr[0].body, { hello: 'world' }); - done(); } + done(); + } + ); + + }); + + it('should operation fail simple function operation run erring', function (done) { + + boundConnectors( + [ + { + id: 'testID', + header: { + message: 'test_fail_op' + }, + body: { + hello: 'world' + } + } + ], + { + + }, + function (err, resArr) { + if (err) { + assert.fail(resArr); + } else { + assert(resArr[0].header.error); + assert.deepEqual(resArr[0].body.message, 'Throw error.'); + } + done(); + } + ); + + }); + + it('should operation fail simple promise function operation run rejecting', function (done) { + + boundConnectors( + [ + { + id: 'testID', + header: { + message: 'test_fail_op_promise' + }, + body: { + hello: 'world' + } + } + ], + { + + }, + function (err, resArr) { + if (err) { + assert.fail(resArr); + } else { + assert(resArr[0].header.error); + assert.deepEqual( + resArr[0].body, + { + code: '#connector_error', + message: 'Reject error' + } + ); + } + done(); } ); }); + //Timeout tests var timeoutConnectors = bindConnectors( [ { @@ -168,8 +256,8 @@ describe.only('#bindConnectors', function () { assert.fail(err); } else { assert.deepEqual(resArr[0].body, { hello: 'world' }); - done(); } + done(); } ); @@ -197,14 +285,14 @@ describe.only('#bindConnectors', function () { assert.fail(err); } else { assert.deepEqual(resArr[0].body, { hello: 'world' }); - done(); } + done(); } ); }); - it('should return timeout error for operation not ending within time limit', function (done) { + it('should error on lambda function level for operation not ending within time limit', function (done) { this.timeout(25000); From 46f64e0556a9b9315b1c98c74a688fd92a1606d3 Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Wed, 9 Jan 2019 17:03:23 +0000 Subject: [PATCH 6/8] removed only from test --- test/bindConnectors/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bindConnectors/index.js b/test/bindConnectors/index.js index 33bd1cf9..3c48021d 100644 --- a/test/bindConnectors/index.js +++ b/test/bindConnectors/index.js @@ -4,7 +4,7 @@ global.when = require('when'); var bindConnectors = require('../../lib/bindConnectors'); /* eslint-disable no-console */ -describe.only('#bindConnectors', function () { +describe('#bindConnectors', function () { global.falafel = {}; From 2db138eb7ae774121515dc3b3e6a369179d76213 Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Wed, 9 Jan 2019 17:05:58 +0000 Subject: [PATCH 7/8] console warn includes op name --- lib/bindConnectors/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index 85a1fb41..668cf46a 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -122,7 +122,7 @@ module.exports = function (config, options) { //Perform this catch only if it is due to timeout .catch(when.TimeoutError, function () { // eslint-disable-next-line no-console - console.warn('The promise has not been closed within the time limit.'); + console.warn('The promise has not been closed within the time limit.', event.header.message); //NOTE: this reject will force this lambda function to error (not the operation only) return when.reject(formatError( { From cb88569b385f3695c7436267fbcadab1191f3a96 Mon Sep 17 00:00:00 2001 From: Johnbastian Date: Wed, 9 Jan 2019 17:33:21 +0000 Subject: [PATCH 8/8] Reduced cut off time from 10 to 5s --- lib/bindConnectors/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/bindConnectors/index.js b/lib/bindConnectors/index.js index 668cf46a..39ea2ba1 100644 --- a/lib/bindConnectors/index.js +++ b/lib/bindConnectors/index.js @@ -29,7 +29,7 @@ module.exports = function (config, options) { /* If context.getRemainingTimeInMillis is available, set it's value - minus 10 seconds, and make sure it is positive; else set the + minus 5 seconds, and make sure it is positive; else set the timeout to 0 (such that it is falsy) */ var timeRemaining = ( @@ -37,7 +37,7 @@ module.exports = function (config, options) { context.getRemainingTimeInMillis() : 0 ); - var promiseTimeout = timeRemaining - 10000; + var promiseTimeout = timeRemaining - 5000; promiseTimeout = ( promiseTimeout < 1 ? 0 : promiseTimeout );