Merge pull request #351 from splitio/error_logs_and_axios

Maintenance patch - #350, #335 and #337 plus dependency updates

CHANGES.txt

@@ -1,3 +1,9 @@
+10.10.1 (Jan XX, 2020)
+ - Updated logging logic to avoid calling console.error in the client side.
+ - Updated logging error messages priority to significantly reduce the amount of error level logs that can't be silenced.
+ - Updated axios to 0.19.2 and few other minor updates on dev dependencies.
+ - Bugfixing - Emit SDK_UPDATE in localhost only when mock was actually updated.
+
 10.10.0 (Jan 10, 2020)
  - Added SDK_READY_FROM_CACHE event, which will be emitted in LocalStorage mode when the SDK finds split cache in the storage of the browser.
  - Updated a dependency to fix a vulnerability (serialize-javascript).

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio",
-  "version": "10.10.0",
+  "version": "10.10.1-canary.0",
   "description": "Split SDK",
   "files": [
     "README.md",
@@ -32,9 +32,9 @@
     "node": ">=6"
   },
   "dependencies": {
-    "@babel/runtime-corejs3": "^7.6.3",
-    "@types/node": "^13.1.6",
-    "axios": "0.19.1",
+    "@babel/runtime-corejs3": "^7.8.3",
+    "@types/node": "^13.1.8",
+    "axios": "0.19.2",
     "core-js": "3.6.1",
     "ip": "1.1.5",
     "js-yaml": "3.13.1",
@@ -45,17 +45,17 @@
     "ioredis": "^4.14.1"
   },
   "devDependencies": {
-    "@babel/cli": "^7.7.7",
-    "@babel/core": "^7.7.7",
-    "@babel/plugin-transform-runtime": "^7.7.6",
-    "@babel/preset-env": "^7.7.7",
-    "@babel/register": "^7.7.7",
+    "@babel/cli": "^7.8.3",
+    "@babel/core": "^7.8.3",
+    "@babel/plugin-transform-runtime": "^7.8.3",
+    "@babel/preset-env": "^7.8.3",
+    "@babel/register": "^7.8.3",
     "axios-mock-adapter": "^1.17.0",
     "babel-loader": "^8.0.6",
-    "copyfiles": "^2.1.1",
+    "copyfiles": "^2.2.0",
     "cross-env": "^6.0.3",
     "csv-streamify": "4.0.0",
-    "eslint": "^6.5.1",
+    "eslint": "^6.8.0",
     "karma": "^4.4.1",
     "karma-chrome-launcher": "^3.1.0",
     "karma-tap": "4.1.4",
@@ -66,12 +66,12 @@
     "redis-dump": "0.1.10",
     "redis-server": "1.2.2",
     "rimraf": "^3.0.0",
-    "sinon": "^8.0.2",
+    "sinon": "^8.1.1",
     "tap-summary": "4.0.0",
     "tape": "4.13.0",
     "tape-catch": "1.0.6",
-    "webpack": "^4.41.2",
-    "webpack-cli": "^3.3.9",
+    "webpack": "^4.41.5",
+    "webpack-cli": "^3.3.10",
     "webpack-merge": "^4.2.2"
   },
   "scripts": {

src/__tests__/offline/browser.spec.js

@@ -40,6 +40,14 @@ const configMocks = () => {
 
 tape('Browser offline mode', function (assert) {
   configMocks();
+  const originalFeaturesMap = {
+    testing_split: 'on',
+    testing_split_with_config: {
+      treatment: 'off',
+      config: '{ "color": "blue" }'
+    }
+  };
+
   const config = {
     core: {
       authorizationKey: 'localhost'
@@ -48,18 +56,12 @@ tape('Browser offline mode', function (assert) {
       impressionsRefreshRate: 0.01,
       eventsPushRate: 0.01,
       metricsRefreshRate: 0.01,
-      offlineRefreshRate: 3
+      offlineRefreshRate: 0.19
     },
     startup: {
       eventsFirstPushWindow: 0
     },
-    features: {
-      testing_split: 'on',
-      testing_split_with_config: {
-        treatment: 'off',
-        config: '{ "color": "blue" }'
-      }
-    }
+    features: originalFeaturesMap
   };
   const factory = SplitFactory(config);
   const manager = factory.manager();
@@ -75,7 +77,9 @@ tape('Browser offline mode', function (assert) {
   assert.equal(client.getTreatment('testing_split'), 'control');
   assert.equal(manager.splits().length, 0);
 
-  client.on(client.Event.SDK_READY, function () {
+  client.once(client.Event.SDK_READY, function () {
+    const readyTimestamp = Date.now();
+
     // Check the information through the client original instance
     assert.equal(client.getTreatment('testing_split'), 'on');
     assert.equal(client.getTreatment('testing_split_2'), 'control');
@@ -139,18 +143,28 @@ tape('Browser offline mode', function (assert) {
       testing_split_with_config: { treatment: 'off', config: '{ "color": "blue" }' }
     });
 
-    // Update the features.
-    factory.settings.features = {
-      testing_split: 'on',
-      testing_split_2: 'off',
-      testing_split_3: 'custom_treatment',
-      testing_split_with_config: {
-        treatment: 'nope',
-        config: null
-      }
-    };
-    // We allow the SDK to process the feature changes and then test again..
-    setTimeout(function () {
+    setTimeout(() => {
+      // Update the features.
+      factory.settings.features = {
+        testing_split: 'on',
+        testing_split_2: 'off',
+        testing_split_3: 'custom_treatment',
+        testing_split_with_config: {
+          treatment: 'nope',
+          config: null
+        }
+      };
+    }, 1000);
+
+    setTimeout(() => { factory.settings.features = originalFeaturesMap; }, 200);
+    setTimeout(() => { factory.settings.features = { testing_split: 'on', testing_split_with_config: { treatment: 'off', config: '{ "color": "blue" }' }};}, 400);
+    setTimeout(() => { factory.settings.features = originalFeaturesMap; }, 600);
+    setTimeout(() => { factory.settings.features = { testing_split: 'on', testing_split_with_config: { treatment: 'off', config: '{ "color": "blue" }' }};}, 750);
+
+    // once updated, test again.
+    client.once(client.Event.SDK_UPDATE, function () {
+      assert.true((Date.now() - readyTimestamp) > 1000, 'Should only emit SDK_UPDATE after a real update.');
+
       assert.equal(client.getTreatment('testing_split_2'), 'off');
       assert.equal(client.getTreatment('testing_split_3'), 'custom_treatment');
       assert.deepEqual(client.getTreatmentWithConfig('testing_split_3'), { treatment: 'custom_treatment', config: null });

src/__tests__/offline/node.spec.js

@@ -49,7 +49,7 @@ const settingsGenerator = mockFileName => {
       impressionsRefreshRate: 0.01,
       eventsPushRate: 0.01,
       metricsRefreshRate: 0.01,
-      offlineRefreshRate: 3
+      offlineRefreshRate: 0.3
     },
     startup: {
       eventsFirstPushWindow: 0,
@@ -64,8 +64,8 @@ const settingsGenerator = mockFileName => {
 tape('NodeJS Offline Mode', function (t) {
 
   t.test('Old format evaluations - .split', DotSplitTests);
-  t.test('New format evaluations - .yaml extension', DotYAMLTests.bind(null, 'split.yaml'));
-  t.test('New format evaluations - .yml extension', DotYAMLTests.bind(null, 'split2.yml'));
+  t.test('New format evaluations - .yaml extension', DotYAMLTests.bind(null, 'split', 'yaml'));
+  t.test('New format evaluations - .yml extension', DotYAMLTests.bind(null, 'split2', 'yml'));
 
   t.test('Old format manager - .split extension', ManagerDotSplitTests);
   t.test('New format manager - .yaml extension', ManagerDotYamlTests.bind(null, 'split.yaml'));
@@ -149,15 +149,24 @@ function DotSplitTests (assert) {
       testing_not_exist: { treatment: 'control', config: null }
     });
 
-    networkAssertions(client, assert).then(() => {
-      client.destroy().then(assert.end);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, '.split'); }, 290);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, '.split'); }, 590);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, '.split'); }, 890);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, 'update.split'); }, 1000);
+
+    client.once(client.Event.SDK_UPDATE, () => {
+      assert.equal(client.getTreatment('qa-user', 'testing_split4'), 'updated_treatment');
+
+      networkAssertions(client, assert).then(() => {
+        client.destroy().then(assert.end);
+      });
     });
   });
 }
 
-function DotYAMLTests (mockFileName, assert) {
+function DotYAMLTests (mockFileName, mockFileExt, assert) {
   configMocks();
-  const config = settingsGenerator(mockFileName);
+  const config = settingsGenerator(`${mockFileName}.${mockFileExt}`);
   const factory = SplitFactory(config);
   const client = factory.client();
   // Tracking some events to test they are not flushed.
@@ -213,8 +222,20 @@ function DotYAMLTests (mockFileName, assert) {
       testing_not_exist: { treatment: 'control', config: null }
     });
 
-    networkAssertions(client, assert).then(() => {
-      client.destroy().then(assert.end);
+    let readyTimestamp = Date.now();
+
+    setTimeout(() => { factory.settings.features = path.join(__dirname, `${mockFileName}.${mockFileExt}` ); }, 290);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, `${mockFileName}.${mockFileExt}` ); }, 590);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, `${mockFileName}.${mockFileExt}` ); }, 890);
+    setTimeout(() => { factory.settings.features = path.join(__dirname, `update.${mockFileName}.${mockFileExt}`); }, 1000);
+
+    client.once(client.Event.SDK_UPDATE, () => {
+      assert.equal(client.getTreatment('qa-user', 'testing_split_update'), 'updated_treatment', 'the update should be properly processed');
+      assert.true((Date.now() - readyTimestamp) > 1000);
+
+      networkAssertions(client, assert).then(() => {
+        client.destroy().then(assert.end);
+      });
     });
   });
 }
@@ -249,7 +270,9 @@ function ManagerDotSplitTests(assert) {
 
     assert.deepEqual(manager.splits(), [expectedView1, expectedView2, expectedView3]);
 
-    client.destroy().then(assert.end);
+    networkAssertions(client, assert).then(() => {
+      client.destroy().then(assert.end);
+    });
   });
 }
 
@@ -305,6 +328,8 @@ function ManagerDotYamlTests(mockFileName, assert) {
 
     assert.deepEqual(manager.splits(), [expectedView1, expectedView2, expectedView3, expectedView4]);
 
-    client.destroy().then(assert.end);
+    networkAssertions(client, assert).then(() => {
+      client.destroy().then(assert.end);
+    });
   });
 }

src/__tests__/offline/update.split

@@ -0,0 +1,8 @@
+####
+# This file a simple mock for testing
+####
+
+testing_split on
+testing_split2 off
+testing_split3 custom_treatment
+testing_split4 updated_treatment

src/__tests__/offline/update.split.yaml

@@ -0,0 +1,21 @@
+# Always on
+- testing_split_on:
+    treatment: "on"
+# This one will (or should) return control for non specified keys on whitelist
+- testing_split_only_wl:
+    treatment: "whitelisted"
+    keys: ["key_for_wl"]
+# Playing with whitelists
+- testing_split_with_wl:
+    treatment: "not_in_whitelist"
+    config: "{\"color\": \"green\"}"
+- testing_split_with_wl:
+    treatment: "one_key_wl"
+    keys: "key_for_wl"
+- testing_split_with_wl:
+    treatment: "multi_key_wl"
+    keys: ["key_for_wl_1", "key_for_wl_2"]
+    config: "{\"color\": \"brown\"}"
+# Update test
+- testing_split_update:
+    treatment: "updated_treatment"

src/__tests__/offline/update.split.yml

@@ -0,0 +1,21 @@
+# Always on
+- testing_split_on:
+    treatment: "on"
+# This one will (or should) return control for non specified keys on whitelist
+- testing_split_only_wl:
+    treatment: "whitelisted"
+    keys: ["key_for_wl"]
+# Playing with whitelists
+- testing_split_with_wl:
+    treatment: "not_in_whitelist"
+    config: "{\"color\": \"green\"}"
+- testing_split_with_wl:
+    treatment: "one_key_wl"
+    keys: "key_for_wl"
+- testing_split_with_wl:
+    treatment: "multi_key_wl"
+    keys: ["key_for_wl_1", "key_for_wl_2"]
+    config: "{\"color\": \"brown\"}"
+# Update test
+- testing_split_update:
+    treatment: "updated_treatment"

src/__tests__/offline/update.split2.yaml

@@ -0,0 +1,21 @@
+# Always on
+- testing_split_on:
+    treatment: "on"
+# This one will (or should) return control for non specified keys on whitelist
+- testing_split_only_wl:
+    treatment: "whitelisted"
+    keys: ["key_for_wl"]
+# Playing with whitelists
+- testing_split_with_wl:
+    treatment: "not_in_whitelist"
+    config: "{\"color\": \"green\"}"
+- testing_split_with_wl:
+    treatment: "one_key_wl"
+    keys: "key_for_wl"
+- testing_split_with_wl:
+    treatment: "multi_key_wl"
+    keys: ["key_for_wl_1", "key_for_wl_2"]
+    config: "{\"color\": \"brown\"}"
+# Update test
+- testing_split_update:
+    treatment: "updated_treatment"

src/__tests__/offline/update.split2.yml

@@ -0,0 +1,21 @@
+# Always on
+- testing_split_on:
+    treatment: "on"
+# This one will (or should) return control for non specified keys on whitelist
+- testing_split_only_wl:
+    treatment: "whitelisted"
+    keys: ["key_for_wl"]
+# Playing with whitelists
+- testing_split_with_wl:
+    treatment: "not_in_whitelist"
+    config: "{\"color\": \"green\"}"
+- testing_split_with_wl:
+    treatment: "one_key_wl"
+    keys: "key_for_wl"
+- testing_split_with_wl:
+    treatment: "multi_key_wl"
+    keys: ["key_for_wl_1", "key_for_wl_2"]
+    config: "{\"color\": \"brown\"}"
+# Update test
+- testing_split_update:
+    treatment: "updated_treatment"

src/producer/updater/SplitChangesFromObject.js

@@ -35,8 +35,8 @@ function FromObjectUpdaterFactory(Fetcher, context) {
       log.error(`There was an issue loading the mock Splits data, no changes will be applied to the current cache. ${err}`);
     }
 
-    if (!loadError) {
-      log.debug('Splits data:');
+    if (!loadError && splitsMock) {
+      log.debug('Splits data: ');
       log.debug(JSON.stringify(splitsMock));
 
       forOwn(splitsMock, function(val, name) {

src/readiness/statusManager.js

@@ -1,5 +1,5 @@
 import logFactory from '../utils/logger';
-const log = logFactory('', { displayAllErrors: true });
+const log = logFactory('');
 
 const NEW_LISTENER_EVENT = 'newListener';
 const REMOVE_LISTENER_EVENT = 'removeListener';