prepare 5.4.0 release (#118)

launchdarkly · Aug 30, 2018 · 3b9f78f · 3b9f78f
1 parent af02269
commit 3b9f78f
Show file tree

Hide file tree

Showing 11 changed files with 540 additions and 148 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,13 @@
 
 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.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.
+
+### Fixed:
+- Evaluating a prerequisite feature flag did not produce an analytics event if the prerequisite flag was off.
+
 ## [5.3.2] - 2018-08-29
 ### Fixed:
 - Fixed TypeScript syntax errors in `index.d.ts`. We are now running the TypeScript compiler in our automated builds to avoid such problems. (Thanks, [PsychicCat](https://github.com/launchdarkly/node-client/pull/116)!)

diff --git a/evaluate_flag.js b/evaluate_flag.js
@@ -8,90 +8,93 @@ var builtins = ['key', 'ip', 'country', 'email', 'firstName', 'lastName', 'avata
 
 var noop = function(){};
 
+// Callback receives (err, detail, events) where detail has the properties "value", "variationIndex", and "reason";
+// detail will never be null even if there's an error.
 function evaluate(flag, user, featureStore, cb) {
   cb = cb || noop;
   if (!user || user.key === null || user.key === undefined) {
-    cb(null, null, null, null);
+    cb(null, errorResult('USER_NOT_SPECIFIED'), []);
     return;
   }
 
   if (!flag) {
-    cb(null, null, null, null);
+    cb(null, errorResult('FLAG_NOT_FOUND'), []);
     return;
   }
 
+  var events = [];
+  evalInternal(flag, user, featureStore, events, function(err, detail) {
+    cb(err, detail, events);
+  });
+}
+
+function evalInternal(flag, user, featureStore, events, cb) {
+  // If flag is off, return the off variation
   if (!flag.on) {
-    // Return the off variation if defined and valid
-    cb(null, flag.offVariation, getVariation(flag, flag.offVariation), null);
+    getOffResult(flag, { kind: 'OFF' }, function(err, detail) {
+      cb(err, detail);
+    });
     return;
   }
 
-  evalInternal(flag, user, featureStore, [], function(err, variation, value, events) {
-    if (err) {
-      cb(err, variation, value, events);
-      return;
-    }
-
-    if (variation === null) {
-      // Return the off variation if defined and valid
-      cb(null, flag.offVariation, getVariation(flag, flag.offVariation), events);
+  checkPrerequisites(flag, user, featureStore, events, function(err, failureReason) {
+    if (err != null || failureReason != null) {
+      getOffResult(flag, failureReason, cb);
     } else {
-      cb(err, variation, value, events);
+      evalRules(flag, user, featureStore, cb);
     }
   });
-  return;
 }
 
-function evalInternal(flag, user, featureStore, events, cb) {
-  // Evaluate prerequisites, if any
+// Callback receives (err, reason) where reason is null if successful, or a "prerequisite failed" reason
+function checkPrerequisites(flag, user, featureStore, events, cb) {
   if (flag.prerequisites) {
     async.mapSeries(flag.prerequisites, 
       function(prereq, callback) {
         featureStore.get(dataKind.features, prereq.key, function(f) {
           // If the flag does not exist in the store or is not on, the prerequisite
           // is not satisfied
-          if (!f || !f.on) {
-            callback(new Error("Unsatisfied prerequisite"), null);
+          if (!f) {
+            callback({ key: prereq.key, err: new Error("Could not retrieve prerequisite feature flag \"" + prereq.key + "\"") });
             return;
           }
-          evalInternal(f, user, featureStore, events, function(err, variation, value) {
+          evalInternal(f, user, featureStore, events, function(err, detail) {
             // If there was an error, the value is null, the variation index is out of range, 
             // or the value does not match the indexed variation the prerequisite is not satisfied
-            events.push(createFlagEvent(f.key, f, user, variation, value, null, flag.key));
-            if (err || value === null || variation != prereq.variation) {
-              callback(new Error("Unsatisfied prerequisite"), null)
+            events.push(createFlagEvent(f.key, f, user, detail, null, flag.key, true));
+            if (err) {
+              callback({ key: prereq.key, err: err });
+            } else if (!f.on || detail.variationIndex != prereq.variation) {
+              // Note that if the prerequisite flag is off, we don't consider it a match no matter what its
+              // off variation was. But we still evaluate it and generate an event.
+              callback({ key: prereq.key });
             } else { 
               // The prerequisite was satisfied
-              callback(null, null);
+              callback(null);
             }
           });          
         });
       }, 
-      function(err, results) {
-        // If the error is that prerequisites weren't satisfied, we don't return an error,
-        // because we want to serve the 'offVariation'
-        if (err) {
-          cb(null, null, null, events);
-          return;
-        } 
-        evalRules(flag, user, featureStore, function(e, variation, value) {
-          cb(e, variation, value, events);
-        });
-      })
+      function(errInfo) {
+        if (errInfo) {
+          cb(errInfo.err, { 'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': errInfo.key });
+        } else {
+          cb(null, null);
+        }
+      });
   } else {
-    evalRules(flag, user, featureStore, function(e, variation, value) {
-      cb(e, variation, value, events);
-    });
+    cb(null, null);
   }
 }
 
+// Callback receives (err, detail)
 function evalRules(flag, user, featureStore, cb) {
   var i, j;
   var target;
   var variation;
   var rule;
   // Check target matches
-  for (i = 0; i < flag.targets.length; i++) {
+  for (i = 0; i < (flag.targets || []).length; i++) {
     target = flag.targets[i];
 
     if (!target.values) {
@@ -100,32 +103,30 @@ function evalRules(flag, user, featureStore, cb) {
 
     for (j = 0; j < target.values.length; j++) {
       if (user.key === target.values[j]) {
-        value = getVariation(flag, target.variation);
-        cb(value === null ? new Error("Undefined variation for flag " + flag.key) : null,
-          target.variation, value);
+        getVariation(flag, target.variation, { kind: 'TARGET_MATCH' }, cb);
         return;
       }
     }
   }
 
-  async.mapSeries(flag.rules,
+  i = 0;
+  async.mapSeries(flag.rules || [],
     function(rule, callback) {
       ruleMatchUser(rule, user, featureStore, function(matched) {
-        setImmediate(callback, matched ? rule : null, null);
+        var match = matched ? { index: i, rule: rule } : null;
+        setImmediate(callback, match, null);
       });
     },
     function(err, results) {
       // we use the "error" value to indicate that a rule was successfully matched (since we only care
       // about the first match, and mapSeries terminates on the first "error")
       if (err) {
-        var rule = err;
-        variation = variationForUser(rule, user, flag);
+        var reason = { kind: 'RULE_MATCH', ruleIndex: err.index, ruleId: err.rule.id };
+        getResultForVariationOrRollout(err.rule, user, flag, reason, cb);
       } else {
         // no rule matched; check the fallthrough
-        variation = variationForUser(flag.fallthrough, user, flag);
+        getResultForVariationOrRollout(flag.fallthrough, user, flag, { kind: 'FALLTHROUGH' }, cb);
       }
-      cb(variation === null ? new Error("Undefined variation for flag " + flag.key) : null,
-          variation, getVariation(flag, variation));
     }
   );
 }
@@ -255,16 +256,39 @@ function matchAny(matchFn, value, values) {
   return false;
 }
 
-// Given an index, return the variation value, or null if 
-// the index is invalid
-function getVariation(flag, index) {
-  if (index === null || index === undefined || index >= flag.variations.length) {
-    return null;
+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')));
   } else {
-    return flag.variations[index];
+    cb(null, { value: flag.variations[index], variationIndex: index, reason: reason });
   }
 }
 
+function getOffResult(flag, reason, cb) {
+  if (flag.offVariation === null || flag.offVariation === undefined) {
+    cb(null, { value: null, variationIndex: null, reason: reason });
+  } else {
+    getVariation(flag, flag.offVariation, reason, cb);
+  }
+}
+
+function getResultForVariationOrRollout(r, user, flag, reason, cb) {
+  if (!r) {
+    cb(new Error('Fallthrough variation undefined'), errResult('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'));
+    } else {
+      getVariation(flag, index, reason, cb);
+    }
+  }
+}
+
+function errorResult(errorKind) {
+  return { value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: errorKind }};
+}
+
 // Given a variation or rollout 'r', select
 // the variation for the given user
 function variationForUser(r, user, flag) {
@@ -337,20 +361,24 @@ function bucketableStringValue(value) {
   return null;
 }
 
-function createFlagEvent(key, flag, user, variation, value, defaultVal, prereqOf) {
-  return {
+function createFlagEvent(key, flag, user, detail, defaultVal, prereqOf, includeReason) {
+  var e = {
     "kind": "feature",
     "key": key,
     "user": user,
-    "variation": variation,
-    "value": value,
+    "variation": detail.variationIndex,
+    "value": detail.value,
     "default": defaultVal,
     "creationDate": new Date().getTime(),
     "version": flag ? flag.version : null,
     "prereqOf": prereqOf,
     "trackEvents": flag ? flag.trackEvents : null,
     "debugEventsUntilDate": flag ? flag.debugEventsUntilDate : null
   };
+  if (includeReason) {
+    e['reason'] = detail.reason;
+  }
+  return e;
 }
 
 module.exports = {evaluate: evaluate, bucketUser: bucketUser, createFlagEvent: createFlagEvent};
diff --git a/event_processor.js b/event_processor.js
@@ -59,6 +59,9 @@ function EventProcessor(sdkKey, config, errorReporter) {
         if (event.version) {
           out.version = event.version;
         }
+        if (event.reason) {
+          out.reason = event.reason;
+        }
         if (config.inlineUsersInEvents || debug) {
           out.user = userFilter.filterUser(event.user);
         } else {

diff --git a/flags_state.js b/flags_state.js
@@ -4,7 +4,7 @@ function FlagsStateBuilder(valid) {
   var flagValues = {};
   var flagMetadata = {};
 
-  builder.addFlag = function(flag, value, variation) {
+  builder.addFlag = function(flag, value, variation, reason) {
     flagValues[flag.key] = value;
     var meta = {
       version: flag.version,
@@ -16,6 +16,9 @@ function FlagsStateBuilder(valid) {
     if (flag.debugEventsUntilDate !== undefined && flag.debugEventsUntilDate !== null) {
       meta.debugEventsUntilDate = flag.debugEventsUntilDate;
     }
+    if (reason) {
+      meta.reason = reason;
+    }
     flagMetadata[flag.key] = meta;
   };
 
@@ -24,6 +27,9 @@ function FlagsStateBuilder(valid) {
       valid: valid,
       allValues: function() { return flagValues; },
       getFlagValue: function(key) { return flagValues[key]; },
+      getFlagReason: function(key) {
+        return flagMetadata[key] ? flagMetadata[key].reason : null;
+      },
       toJSON: function() {
         return Object.assign({}, flagValues, { $flagsState: flagMetadata, $valid: valid });
       }