prepare 5.4.1 release (launchdarkly#120)

JakeGinnivan · Sep 5, 2018 · daf03e6 · daf03e6
1 parent 3b9f78f
commit daf03e6
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 All notable changes to the LaunchDarkly Node.js SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org).
 
+## [5.4.1] - 2018-09-05
+### Fixed:
+- Fixed a ReferenceError that occurred if a feature flag had invalid properties, e.g. a rule referred to a nonexistent variation index. Instead, an error will be written to the log and the flag will return the default value. ([#119](https://github.com/launchdarkly/node-client/issues/119))
+
 ## [5.4.0] - 2018-08-30
 ### Added:
 - The new `LDClient` method `variationDetail` allows you to evaluate a feature flag (using the same parameters as you would for `variation`) and receive more information about how the value was calculated. This information is returned in an object that contains both the result value and a "reason" object which will tell you, for instance, if the user was individually targeted for the flag or was matched by one of the flag's rules, or if the flag returned the default value due to an error.

diff --git a/evaluate_flag.js b/evaluate_flag.js
@@ -258,7 +258,7 @@ function matchAny(matchFn, value, values) {
 
 function getVariation(flag, index, reason, cb) {
   if (index === null || index === undefined || index < 0 || index >= flag.variations.length) {
-    cb(new Error('Invalid variation index in flag', errResult('MALFORMED_FLAG')));
+    cb(new Error('Invalid variation index in flag'), errorResult('MALFORMED_FLAG'));
   } else {
     cb(null, { value: flag.variations[index], variationIndex: index, reason: reason });
   }
@@ -274,11 +274,11 @@ function getOffResult(flag, reason, cb) {
 
 function getResultForVariationOrRollout(r, user, flag, reason, cb) {
   if (!r) {
-    cb(new Error('Fallthrough variation undefined'), errResult('MALFORMED_FLAG'));
+    cb(new Error('Fallthrough variation undefined'), errorResult('MALFORMED_FLAG'));
   } else {
     var index = variationForUser(r, user, flag);
     if (index === null) {
-      cb(new Error('Variation/rollout object with no variation or rollout'), errResult('MALFORMED_FLAG'));
+      cb(new Error('Variation/rollout object with no variation or rollout'), errorResult('MALFORMED_FLAG'));
     } else {
       getVariation(flag, index, reason, cb);
     }

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "ldclient-node",
-  "version": "5.4.0",
+  "version": "5.4.1",
   "description": "LaunchDarkly SDK for Node.js",
   "main": "index.js",
   "scripts": {

diff --git a/test/evaluate_flag-test.js b/test/evaluate_flag-test.js
@@ -28,6 +28,21 @@ function evalBooleanFlag(flag, user, cb) {
   });
 }
 
+function makeFlagWithRules(rules, fallthrough) {
+  if (!fallthrough) {
+    fallthrough = { variation: 0 };
+  }
+  return {
+    key: 'feature',
+    on: true,
+    rules: rules,
+    targets: [],
+    fallthrough: fallthrough,
+    offVariation: 1,
+    variations: ['a', 'b', 'c']
+  };
+}
+
 function makeBooleanFlagWithOneClause(clause) {
   return {
       key: 'feature',
@@ -80,24 +95,99 @@ describe('evaluate', function() {
     });
   });
 
-  it('returns fallthrough if flag is on and there are no rules', function(done) {
+  it('returns error if off variation is too high', function(done) {
     var flag = {
       key: 'feature',
-      on: true,
-      rules: [],
-      targets: [],
-      offVariation: null,
+      on: false,
+      offVariation: 99,
+      fallthrough: { variation: 0 },
+      variations: ['a', 'b', 'c']
+    };
+    var user = { key: 'x' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Invalid variation index in flag'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' } });
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if off variation is negative', function(done) {
+    var flag = {
+      key: 'feature',
+      on: false,
+      offVariation: -1,
       fallthrough: { variation: 0 },
       variations: ['a', 'b', 'c']
     };
     var user = { key: 'x' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Invalid variation index in flag'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' } });
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns fallthrough variation if flag is on and no rules match', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['other'] }], variation: 2 };
+    var flag = makeFlagWithRules([rule], { variation: 0 });
+    var user = { key: 'x' };
     evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
       expect(detail).toMatchObject({ value: 'a', variationIndex: 0, reason: { kind: 'FALLTHROUGH' } });
       expect(events).toMatchObject([]);
       done();
     });
   });
 
+  it('returns error if fallthrough variation is too high', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['other'] }], variation: 99 };
+    var flag = makeFlagWithRules([rule], { variation: 99 });
+    var user = { key: 'x' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Invalid variation index in flag'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if fallthrough variation is negative', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['other'] }], variation: 99 };
+    var flag = makeFlagWithRules([rule], { variation: -1 });
+    var user = { key: 'x' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Invalid variation index in flag'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if fallthrough has no variation or rollout', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['other'] }], variation: 99 };
+    var flag = makeFlagWithRules([rule], { });
+    var user = { key: 'x' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Variation/rollout object with no variation or rollout'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if fallthrough has rollout with no variations', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['other'] }], variation: 99 };
+    var flag = makeFlagWithRules([rule], { rollout: { variations: [] } });
+    var user = { key: 'x' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Variation/rollout object with no variation or rollout'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
   it('returns off variation if prerequisite is not found', function(done) {
     var flag = {
       key: 'feature0',
@@ -223,27 +313,8 @@ describe('evaluate', function() {
   });
 
   it('matches user from rules', function(done) {
-    var flag = {
-      key: 'feature0',
-      on: true,
-      rules: [
-        {
-          id: 'id',
-          clauses: [
-            {
-              attribute: 'key',
-              op: 'in',
-              values: ['userkey']
-            }
-          ],
-          variation: 2
-        }
-      ],
-      targets: [],
-      fallthrough: { variation: 0 },
-      offVariation: 1,
-      variations: ['a', 'b', 'c']
-    };
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['userkey'] }], variation: 2 };
+    var flag = makeFlagWithRules([rule]);
     var user = { key: 'userkey' };
     evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
       expect(detail).toMatchObject({ value: 'c', variationIndex: 2,
@@ -253,6 +324,54 @@ describe('evaluate', function() {
     });
   });
 
+  it('returns error if rule variation is too high', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['userkey'] }], variation: 99 };
+    var flag = makeFlagWithRules([rule]);
+    var user = { key: 'userkey' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Invalid variation index in flag'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if rule variation is negative', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['userkey'] }], variation: -1 };
+    var flag = makeFlagWithRules([rule]);
+    var user = { key: 'userkey' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Invalid variation index in flag'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if rule has no variation or rollout', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['userkey'] }] };
+    var flag = makeFlagWithRules([rule]);
+    var user = { key: 'userkey' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Variation/rollout object with no variation or rollout'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
+  it('returns error if rule has rollout with no variations', function(done) {
+    var rule = { id: 'id', clauses: [{ attribute: 'key', op: 'in', values: ['userkey'] }], rollout: { variations: [] } };
+    var flag = makeFlagWithRules([rule]);
+    var user = { key: 'userkey' };
+    evaluate.evaluate(flag, user, featureStore, function(err, detail, events) {
+      expect(err).toEqual(Error('Variation/rollout object with no variation or rollout'));
+      expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: 'MALFORMED_FLAG' }});
+      expect(events).toMatchObject([]);
+      done();
+    });
+  });
+
   it('matches user from targets', function(done) {
     var flag = {
       key: 'feature0',