feat(pathError hook): add pathError hook for errors identified while

merging jsonGraphs if a route returns a jsonGraph that has a node inside of it that is of `$type: 'error'`, then during the merge process the `pathError` hook will be called. Just add a `hooks.pathError` function to the options argument to `Router` ctor: ```js var router = new Router(routes, { hooks: { pathError: function (pathValue) { } } }); ```
Netflix · Jan 20, 2017 · 816eccc · 816eccc
1 parent 3049aa5
commit 816eccc
Show file tree

Hide file tree

Showing 10 changed files with 124 additions and 8 deletions.
diff --git a/src/Router.js b/src/Router.js
@@ -14,6 +14,7 @@ var Router = function(routes, options) {
     this._rst = parseTree(routes);
     this._matcher = matcher(this._rst);
     this._debug = opts.debug;
+    this._pathErrorHook = (opts.hooks && opts.hooks.pathError) || noOp;
     this._errorHook = (opts.hooks && opts.hooks.error) || noOp;
     this.maxRefFollow = opts.maxRefFollow || MAX_REF_FOLLOW;
     this.maxPaths = opts.maxPaths || MAX_PATHS;

diff --git a/src/cache/jsongMerge.js b/src/cache/jsongMerge.js
@@ -1,21 +1,23 @@
 var iterateKeySet = require('falcor-path-utils').iterateKeySet;
 var types = require('./../support/types');
 var $ref = types.$ref;
+var $error = types.$error;
 var clone = require('./../support/clone');
 var cloneArray = require('./../support/cloneArray');
 var catAndSlice = require('./../support/catAndSlice');
 
 /**
  * merges jsong into a seed
  */
-module.exports = function jsongMerge(cache, jsongEnv) {
+module.exports = function jsongMerge(cache, jsongEnv, routerInstance) {
     var paths = jsongEnv.paths;
     var j = jsongEnv.jsonGraph;
     var references = [];
     var values = [];
 
     paths.forEach(function(p) {
         merge({
+            router: routerInstance,
             cacheRoot: cache,
             messageRoot: j,
             references: references,
@@ -49,6 +51,12 @@ function merge(config, cache, message, depth, path, fromParent, fromKey) {
     if (message === null || typeOfMessage !== 'object' || message.$type) {
         fromParent[fromKey] = clone(message);
 
+        // If we notice an error while merging, we'll fire the error hook
+        // for logging purposes.
+        if (message && message.$type === $error) {
+            config.router._pathErrorHook({ path: path, value: message });
+        }
+
         // NOTE: If we have found a reference at our cloning position
         // and we have resolved our path then add the reference to
         // the unfulfilledRefernces.

diff --git a/src/router/get.js b/src/router/get.js
@@ -53,7 +53,7 @@ module.exports = function routerGet(paths) {
                             mCGRI(out.jsonGraph, [{
                                 jsonGraph: jsonGraphFragment.jsonGraph,
                                 paths: unhandledPaths
-                            }]);
+                            }], router);
                             return out;
                         }).
                         defaultIfEmpty(out);

diff --git a/src/router/set.js b/src/router/set.js
@@ -137,7 +137,7 @@ module.exports = function routerSet(jsonGraph) {
                             mCGRI(out.jsonGraph, [{
                                 jsonGraph: unhandledJsonGraphEnv.jsonGraph,
                                 paths: unhandledPaths
-                            }]);
+                            }], router);
                             return out;
                         }).
                         defaultIfEmpty(out);

diff --git a/src/run/call/runCallAction.js b/src/run/call/runCallAction.js
@@ -84,7 +84,8 @@ function runCallAction(matchAndPath, routerInstance, callPath, args,
 
                 });
 
-                var invsRefsAndValues = mCGRI(jsongCache, callOutput);
+                var invsRefsAndValues =
+                    mCGRI(jsongCache, callOutput, routerInstance);
                 invsRefsAndValues.references.forEach(function(ref) {
                     refs[++refLen] = ref;
                 });

diff --git a/src/run/mergeCacheAndGatherRefsAndInvalidations.js b/src/run/mergeCacheAndGatherRefsAndInvalidations.js
@@ -14,7 +14,9 @@ module.exports = mergeCacheAndGatherRefsAndInvalidations;
  * @param {Object} cache
  * @param {Array} jsongOrPVs
  */
-function mergeCacheAndGatherRefsAndInvalidations(cache, jsongOrPVs) {
+function mergeCacheAndGatherRefsAndInvalidations(
+    cache, jsongOrPVs, routerInstance
+) {
     var references = [];
     var len = -1;
     var invalidations = [];
@@ -39,7 +41,7 @@ function mergeCacheAndGatherRefsAndInvalidations(cache, jsongOrPVs) {
         }
 
         else if (isJSONG(jsongOrPV)) {
-            refsAndValues = jsongMerge(cache, jsongOrPV);
+            refsAndValues = jsongMerge(cache, jsongOrPV, routerInstance);
         }
 
         // Last option are path values.

diff --git a/src/run/recurseMatchAndExecute.js b/src/run/recurseMatchAndExecute.js
@@ -77,7 +77,8 @@ function _recurseMatchAndExecute(
                         value = [value];
                     }
 
-                    var invsRefsAndValues = mCGRI(jsongCache, value);
+                    var invsRefsAndValues =
+                        mCGRI(jsongCache, value, routerInstance);
                     var invalidations = invsRefsAndValues.invalidations;
                     var unhandled = invsRefsAndValues.unhandledPaths;
                     var messages = invsRefsAndValues.messages;

diff --git a/src/run/set/runSetAction.js b/src/run/set/runSetAction.js
@@ -67,7 +67,7 @@ function runSetAction(routerInstance, jsongMessage, matchAndPath, jsongCache) {
             paths: [match.requested]
         };
         arg = {};
-        jsongMerge(arg, subJsonGraphEnv);
+        jsongMerge(arg, subJsonGraphEnv, routerInstance);
     }
     try {
         out = match.action.call(routerInstance, arg);

diff --git a/test/unit/core/get.spec.js b/test/unit/core/get.spec.js
@@ -619,6 +619,68 @@ describe('Get', function() {
             subscribe(noOp, done, done);
     });
 
+    it('should fire the error hook if the graph  node in it', function (done) {
+        var callCount = 0;
+        var callContext = null;
+        var callArgs = null;
+        var router = new R([{
+            route: 'videos[{integers:ids}].title',
+            get: function (alias) {
+                return Observable.of({
+                    jsonGraph: {
+                        videos: {
+                            1: {
+                                title: { $type: 'error', value: { message: 'bad luck for you' } }
+                            }
+                        }
+                    }
+                });
+            }
+        }],
+        {
+            hooks: {
+                pathError: function () {
+                    callCount++;
+                    callArgs = Array.prototype.slice.call(arguments, 0);
+                    callContext = this;
+                }
+            }
+        });
+
+        router.get([['videos',1,'title']])
+            .do(
+                function (jsonGraph) {
+                    expect(jsonGraph).to.deep.equal({
+                        jsonGraph: {
+                            'videos': {
+                                1: {
+                                    'title': {
+                                        $type: 'error', value: {
+                                            message: 'bad luck for you'
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    })
+                },
+                noOp,
+                function () {
+                    expect(callCount).to.equal(1);
+                    expect(callArgs).to.deep.equal([{
+                        path: ['videos', 1, 'title'],
+                        value: { $type: 'error', value: { message: 'bad luck for you' } }
+                    }]);
+                    expect(callContext).to.equal(router);
+                }
+            )
+            .subscribe(
+                noOp,
+                done,
+                done
+            )
+    });
+
     it('should fire the error hook passed in via options.hooks', function (done) {
       var callCount = 0;
       var callContext = null;

diff --git a/test/unit/internal/jsongMerge.spec.js b/test/unit/internal/jsongMerge.spec.js
@@ -215,6 +215,47 @@ describe('JSONG - Merge', function() {
             }]
         });
     });
+
+    it('should fire the router error hook if $type:"error" is found while walking', function () {
+        var jsong = {
+            jsonGraph: {
+                this: {
+                    is: {
+                        deep: {
+                            yo: { $type: 'error', value: { message: 'lol error' } }
+                        }
+                    }
+                }
+            },
+            paths: [['this', 'is', 'deep', 'yo']]
+        };
+        var callArgs = null;
+        var callCount = 0;
+        var mockRouter = {
+            _pathErrorHook: function () {
+                callCount++;
+                callArgs = Array.prototype.slice.call(arguments);
+            }
+        }
+        var cache = {};
+        var out = jsongMerge(cache, jsong, mockRouter);
+        expect(out).to.deep.equal({
+            values: [{
+                path: ['this', 'is', 'deep', 'yo'],
+                value: { $type: 'error', value: { message: 'lol error' } }
+            }],
+            references: []
+        });
+        expect(callArgs).to.deep.equals([
+            {
+                path: ['this', 'is', 'deep', 'yo'],
+                value : {
+                    $type: 'error', value: { message: 'lol error'}
+                }
+            }
+        ]);
+        expect(callCount).to.equals(1);
+    });
 });
 
 function mergeTest(jsong) {