From 506c802172ea399e81ea8251dfe6e006f904aaa3 Mon Sep 17 00:00:00 2001 From: Andreas Garnaes Date: Wed, 19 Aug 2020 16:00:08 +0200 Subject: [PATCH 1/2] Improve asynchronous failure handling --- lib/fflip.js | 32 ++++++++++++++------------------ test/fflip.js | 42 +++++++++++++++++++++++++++++++++++------- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/lib/fflip.js b/lib/fflip.js index 9a5a5b8..156d673 100644 --- a/lib/fflip.js +++ b/lib/fflip.js @@ -106,22 +106,6 @@ function setFeatures(configVal, callback) { self.features = processUserFeatures(configVal); } -/** - * The callback called by the user-defined function for reloading features. - * - * @param {Object} data - * @param {function} callback - * @return {void} - * @private - */ -function getFeaturesCallback(data, callback) { - self.features = processUserFeatures(data) || self.features; - - if (typeof callback !== 'undefined') { - callback(); - } -} - /** * Sets the reload rate for fetching new features. @@ -239,8 +223,20 @@ var self = module.exports = { return; } - getFeatures(function(data) { - getFeaturesCallback(data, callback); + getFeatures(function(err, data) { + if(typeof err === 'undefined') { + try { + self.features = processUserFeatures(data) || self.features; + } catch(e) { + err = e; + } + } + + if(typeof callback !== 'undefined') { + callback(err); + } else { + throw err; + } }); }, diff --git a/test/fflip.js b/test/fflip.js index 3d543a4..9675fe9 100644 --- a/test/fflip.js +++ b/test/fflip.js @@ -146,7 +146,7 @@ describe('fflip', function(){ it('should set features if given an asyncronous loading function', function(done){ var loadAsyncronously = function(callback) { - callback(configData.features); + callback(undefined, configData.features); assert.deepEqual(fflip.features, { fEmpty: configData.features[0], fOpen: configData.features[1], @@ -163,9 +163,10 @@ describe('fflip', function(){ it('should invoke the callback after asynchronous features have been loaded', function(done){ var loadAsyncronously = function(callback) { - callback(configData.features); + callback(undefined, configData.features); }; - fflip.config({features: loadAsyncronously, criteria: configData.criteria}, function() { + fflip.config({features: loadAsyncronously, criteria: configData.criteria}, function(err) { + assert.equal(err, undefined); assert.deepEqual(fflip.features, { fEmpty: configData.features[0], fOpen: configData.features[1], @@ -179,6 +180,17 @@ describe('fflip', function(){ }); }); + it('should invoke the callback after asynchronous features have failed', function(done){ + var error = new Error(); + var loadAsyncronously = function(callback) { + callback(error); + }; + fflip.config({features: loadAsyncronously, criteria: configData.criteria}, function(err) { + assert.equal(err, error); + done(); + }); + }); + it('should set criteria if given static criteria array', function(){ fflip.criteria = {}; fflip.config(configData); @@ -205,7 +217,7 @@ describe('fflip', function(){ it('should be called every X seconds where X = reloadRate', function(done) { this.timeout(205); var loadAsyncronously = function(callback) { - callback({}); + callback(undefined, {}); done(); }; fflip.config({features: loadAsyncronously, reload: 0.2, criteria: configData.criteria}); @@ -215,7 +227,7 @@ describe('fflip', function(){ this.timeout(100); var testReady = false; var loadAsyncronously = function(callback) { - callback({}); + callback(undefined, {}); if(testReady) done(); }; @@ -227,10 +239,26 @@ describe('fflip', function(){ it('should invoke the callback after asynchronous features have been loaded', function(done) { this.timeout(100); var loadAsyncronously = function(callback) { - callback({}); + callback(undefined, {}); + }; + fflip.config({features: loadAsyncronously, criteria: configData.criteria}); + fflip.reload(function(err) { + assert.equal(err, undefined); + done(); + }); + }); + + it('should invoke the callback after asynchronous features have failed', function(done) { + this.timeout(100); + var error = new Error(); + var testReady = false; + var loadAsyncronously = function(callback) { + callback(testReady ? error : undefined, {}); }; fflip.config({features: loadAsyncronously, criteria: configData.criteria}); - fflip.reload(function() { + testReady = true; + fflip.reload(function(err) { + assert.equal(err, error); done(); }); }); From 39be3e55ce97e25a76399afbad59c3f25b6f83b1 Mon Sep 17 00:00:00 2001 From: Andreas Garnaes Date: Tue, 15 Sep 2020 09:24:01 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Andreas Lind --- lib/fflip.js | 2 +- test/fflip.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/fflip.js b/lib/fflip.js index 156d673..8a5cda5 100644 --- a/lib/fflip.js +++ b/lib/fflip.js @@ -224,7 +224,7 @@ var self = module.exports = { } getFeatures(function(err, data) { - if(typeof err === 'undefined') { + if(!err) { try { self.features = processUserFeatures(data) || self.features; } catch(e) { diff --git a/test/fflip.js b/test/fflip.js index 9675fe9..1b4397d 100644 --- a/test/fflip.js +++ b/test/fflip.js @@ -166,7 +166,7 @@ describe('fflip', function(){ callback(undefined, configData.features); }; fflip.config({features: loadAsyncronously, criteria: configData.criteria}, function(err) { - assert.equal(err, undefined); + assert(!err); assert.deepEqual(fflip.features, { fEmpty: configData.features[0], fOpen: configData.features[1], @@ -243,7 +243,7 @@ describe('fflip', function(){ }; fflip.config({features: loadAsyncronously, criteria: configData.criteria}); fflip.reload(function(err) { - assert.equal(err, undefined); + assert(!err); done(); }); });