fix #300 Undefined pipe expression throws error that doesn't provide

useful info
ariatemplates · Oct 7, 2014 · d06b1bf · d06b1bf
1 parent 37d73b9
commit d06b1bf
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 9 deletions.
diff --git a/hsp/expressions/observable.js b/hsp/expressions/observable.js
@@ -15,6 +15,14 @@
 
 var evaluator = require('./evaluator');
 
+var evaluatorNotNull = function(tree, scope) {
+    var result = evaluator(tree, scope);
+    if (result == null) {
+        throw new Error(tree.v + " is not defined");
+    }
+    return result;
+};
+
 /**
  * Get all the observable pairs for a given expression. Observable pairs
  * are usually input to model-change-observing utilities (ex. Object.observe).
@@ -69,9 +77,11 @@ module.exports = function getObservablePairs(tree, scope) {
             } else {
                 return partialResult;
             }
-        } if (tree.v === '(') { //function call on a scope
+        }
+        if (tree.v === '(') { //function call on a scope
             return [[scope, null]].concat(getObservablePairs(tree.r, scope));
-        } if (tree.v === '[') { //dynamic property access
+        }
+        if (tree.v === '[') { //dynamic property access
             leftValue = evaluator(tree.l, scope);
             if (leftValue) {
                 rightValue = evaluator(tree.r, scope);
@@ -88,14 +98,13 @@ module.exports = function getObservablePairs(tree, scope) {
     } else if (tree.a === 'tnr') {
         partialResult = getObservablePairs(tree.l, scope);
         if (tree.v === '(') { // function call on an object
-            partialResult = partialResult.concat([ [evaluator(tree.l, scope), null]]);
+            partialResult = partialResult.concat([[evaluatorNotNull(tree.l, scope), null]]);
         } else if (tree.v === '|') { // pipe operator is similar to function calls
             partialResult = partialResult.concat(getObservablePairs(tree.r, scope));
-            if (tree.r.v === '.') { // pipe is a function defined on an object
-                partialResult = partialResult.concat([[evaluator(tree.r.l, scope), null]]);
-            } else { // pipe is a function defined on a scope
-                partialResult = partialResult.concat([[evaluator(tree.r, scope), null]]);
-            }
+            var obj = tree.r.v === '.' ?
+                    tree.r.l : // pipe is a function defined on an object
+                    tree.r; // pipe is a function defined on a scope
+            partialResult = partialResult.concat([[evaluatorNotNull(obj, scope), null]]);
         } else {
             partialResult = partialResult.concat(getObservablePairs(tree.r, scope));
         }

diff --git a/hsp/rt/exphandler.js b/hsp/rt/exphandler.js
@@ -132,6 +132,10 @@ var PrattExpr = klass({
     },
 
     getObservablePairs : function (eh, vscope) {
-        return this.bound ? exobservable(this.ast, vscope) : null;
+        try {
+            return this.bound ? exobservable(this.ast, vscope) : null;
+        } catch (e) {
+            log.warning("Error evaluating expression '" + this.exptext + "': " + e.message);
+        }
     }
 });
diff --git a/test/expressions/observable.spec.js b/test/expressions/observable.spec.js
@@ -141,6 +141,23 @@ describe('determining observable pairs', function () {
                     [scope.sort, null]
                 ]);
         });
+
+        it('should report runtime errors for pipes where function is undefined', function () {
+            var scope = {foo: ""};
+            expect(function () {
+                observable(p('foo|bar'), scope);
+            }).to.throwError(function(err){
+                expect(err.message).to.eql('bar is not defined');
+            });
+
+            var scope = {foo: ""};
+            expect(function () {
+                observable(p('foo|bar.baz'), scope);
+            }).to.throwError(function(err){
+                expect(err.message).to.eql('bar is not defined');
+            });
+
+        });
     });
 
     describe('array literals', function () {