From b2d63ac48bdc61b5a4afdd10b8485c0c1ab8cdca Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 16 Nov 2010 13:57:41 -0800 Subject: Changed error handling so that better stack traces are displayed in the ng-errors --- .externalToolBuilders/JSTD_Tests.launch | 2 +- .externalToolBuilders/docs.launch | 2 +- css/angular.css | 1 + src/Angular.js | 13 ++++++ src/Scope.js | 2 +- src/directives.js | 10 ++-- src/parser.js | 83 ++++++++++++--------------------- src/services.js | 27 ++++++++--- test/BinderTest.js | 13 +++--- test/JsonSpec.js | 23 +++++---- test/ParserSpec.js | 16 ++----- test/ScopeSpec.js | 16 +++---- test/directivesSpec.js | 2 +- test/servicesSpec.js | 36 ++++++++++++++ 14 files changed, 144 insertions(+), 102 deletions(-) diff --git a/.externalToolBuilders/JSTD_Tests.launch b/.externalToolBuilders/JSTD_Tests.launch index 439cb407..250aa755 100644 --- a/.externalToolBuilders/JSTD_Tests.launch +++ b/.externalToolBuilders/JSTD_Tests.launch @@ -1,7 +1,7 @@ - + diff --git a/.externalToolBuilders/docs.launch b/.externalToolBuilders/docs.launch index 25b6b881..652ac9bd 100644 --- a/.externalToolBuilders/docs.launch +++ b/.externalToolBuilders/docs.launch @@ -2,7 +2,7 @@ - + diff --git a/css/angular.css b/css/angular.css index a293e8eb..9c1ddedb 100644 --- a/css/angular.css +++ b/css/angular.css @@ -8,6 +8,7 @@ border: 2px solid #FF0000; font-family: "Courier New", Courier, monospace; font-size: smaller; + white-space: pre; } .ng-validation-error { diff --git a/src/Angular.js b/src/Angular.js index 5a5a4ab3..e0cf2df4 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -84,6 +84,7 @@ var _undefined = undefined, PRIORITY_WATCH = -1000, PRIORITY_LAST = 99999, PRIORITY = {'FIRST': PRIORITY_FIRST, 'LAST': PRIORITY_LAST, 'WATCH':PRIORITY_WATCH}, + Error = window.Error, jQuery = window['jQuery'] || window['$'], // weirdness to make IE happy _ = window['_'], /** holds major version number for IE or NaN for real browsers */ @@ -557,6 +558,18 @@ function foreachSorted(obj, iterator, context) { } +function formatError(arg) { + if (arg instanceof Error) { + if (arg.stack) { + arg = arg.stack; + } else if (arg.sourceURL) { + arg = arg.message + '\n' + arg.sourceURL + ':' + arg.line; + } + } + return arg; +} + + function extend(dst) { foreach(arguments, function(obj){ if (obj !== dst) { diff --git a/src/Scope.js b/src/Scope.js index 1f8e81e5..64019be4 100644 --- a/src/Scope.js +++ b/src/Scope.js @@ -101,7 +101,7 @@ function expressionCompile(exp){ } function errorHandlerFor(element, error) { - elementError(element, NG_EXCEPTION, isDefined(error) ? toJson(error) : error); + elementError(element, NG_EXCEPTION, isDefined(error) ? formatError(error) : error); } function createScope(parent, providers, instanceCache) { diff --git a/src/directives.js b/src/directives.js index e359d6cc..2958773d 100644 --- a/src/directives.js +++ b/src/directives.js @@ -182,7 +182,7 @@ angularDirective("ng:bind", function(expression, element){ oldElement = this.hasOwnProperty($$element) ? this.$element : _undefined; this.$element = element; value = this.$tryEval(expression, function(e){ - error = toJson(e); + error = formatError(e); }); this.$element = oldElement; // If we are HTML than save the raw HTML data so that we don't @@ -466,15 +466,15 @@ angularWidget("@ng:repeat", function(expression, element){ var match = expression.match(/^\s*(.+)\s+in\s+(.*)\s*$/), lhs, rhs, valueIdent, keyIdent; if (! match) { - throw "Expected ng:repeat in form of 'item in collection' but got '" + - expression + "'."; + throw Error("Expected ng:repeat in form of 'item in collection' but got '" + + expression + "'."); } lhs = match[1]; rhs = match[2]; match = lhs.match(/^([\$\w]+)|\(([\$\w]+)\s*,\s*([\$\w]+)\)$/); if (!match) { - throw "'item' in 'item in collection' should be identifier or (key, value) but got '" + - keyValue + "'."; + throw Error("'item' in 'item in collection' should be identifier or (key, value) but got '" + + keyValue + "'."); } valueIdent = match[3] || match[1]; keyIdent = match[2]; diff --git a/src/parser.js b/src/parser.js index 85b9c651..992696ee 100644 --- a/src/parser.js +++ b/src/parser.js @@ -67,10 +67,7 @@ function lex(text, parseStringsForObjects){ tokens.push({index:index, text:ch, fn:fn, json: was('[,:') && is('+-')}); index += 1; } else { - throw "Lexer Error: Unexpected next character [" + - text.substring(index) + - "] in expression '" + text + - "' at column '" + (index+1) + "'."; + throwError("Unexpected next character ", index, index+1); } } lastCh = ch; @@ -103,6 +100,16 @@ function lex(text, parseStringsForObjects){ function isExpOperator(ch) { return ch == '-' || ch == '+' || isNumber(ch); } + + function throwError(error, start, end) { + end = end || index; + throw Error("Lexer Error: " + error + " at column" + + (isDefined(start) ? + "s " + start + "-" + index + " [" + text.substring(start, end) + "]" : + " " + end) + + " in expression [" + text + "]."); + } + function readNumber() { var number = ""; var start = index; @@ -121,7 +128,7 @@ function lex(text, parseStringsForObjects){ } else if (isExpOperator(ch) && (!peekCh || !isNumber(peekCh)) && number.charAt(number.length - 1) == 'e') { - throw 'Lexer found invalid exponential value "' + text + '"'; + throwError('Invalid exponent'); } else { break; } @@ -151,7 +158,7 @@ function lex(text, parseStringsForObjects){ } tokens.push({index:start, text:ident, fn:fn, json: OPERATORS[ident]}); } - + function readString(quote) { var start = index; index++; @@ -165,9 +172,7 @@ function lex(text, parseStringsForObjects){ if (ch == 'u') { var hex = text.substring(index + 1, index + 5); if (!hex.match(/[\da-f]{4}/i)) - throw "Lexer Error: Invalid unicode escape [\\u" + - hex + "] starting at column '" + - start + "' in expression '" + text + "'."; + throwError( "Invalid unicode escape [\\u" + hex + "]"); index += 4; string += String.fromCharCode(parseInt(hex, 16)); } else { @@ -194,9 +199,7 @@ function lex(text, parseStringsForObjects){ } index++; } - throw "Lexer Error: Unterminated quote [" + - text.substring(start) + "] starting at column '" + - (start+1) + "' in expression '" + text + "'."; + throwError("Unterminated quote", start); } function readRegexp(quote) { var start = index; @@ -227,9 +230,7 @@ function lex(text, parseStringsForObjects){ } index++; } - throw "Lexer Error: Unterminated RegExp [" + - text.substring(start) + "] starting at column '" + - (start+1) + "' in expression '" + text + "'."; + throwError("Unterminated RegExp", start); } } @@ -249,17 +250,16 @@ function parser(text, json){ }; /////////////////////////////////// - - function error(msg, token) { - throw "Token '" + token.text + - "' is " + msg + " at column='" + - (token.index + 1) + "' of expression '" + - text + "' starting at '" + text.substring(token.index) + "'."; + function throwError(msg, token) { + throw Error("Parse Error: Token '" + token.text + + "' " + msg + " at column " + + (token.index + 1) + " of expression [" + + text + "] starting at [" + text.substring(token.index) + "]."); } function peekToken() { if (tokens.length === 0) - throw "Unexpected end of expression: " + text; + throw Error("Unexpected end of expression: " + text); return tokens[0]; } @@ -280,10 +280,7 @@ function parser(text, json){ if (token) { if (json && !token.json) { index = token.index; - throw "Expression at column='" + - token.index + "' of expression '" + - text + "' starting at '" + text.substring(token.index) + - "' is not valid json."; + throwError("is not valid json", token); } tokens.shift(); this.currentToken = token; @@ -294,11 +291,7 @@ function parser(text, json){ function consume(e1){ if (!expect(e1)) { - var token = peek(); - throw "Expecting '" + e1 + "' at column '" + - (token.index+1) + "' in '" + - text + "' got '" + - text.substring(token.index) + "'."; + throwError("is unexpected, expecting [" + e1 + "]", peek()); } } @@ -320,8 +313,7 @@ function parser(text, json){ function assertAllConsumed(){ if (tokens.length !== 0) { - throw "Did not understand '" + text.substring(tokens[0].index) + - "' while evaluating '" + text + "'."; + throwError("is extra token not part of expression", tokens[0]); } } @@ -387,18 +379,7 @@ function parser(text, json){ } function expression(){ - return throwStmt(); - } - - function throwStmt(){ - if (expect('throw')) { - var throwExp = assignment(); - return function (self) { - throw throwExp(self); - }; - } else { - return assignment(); - } + return assignment(); } function assignment(){ @@ -406,9 +387,8 @@ function parser(text, json){ var token; if (token = expect('=')) { if (!left.isAssignable) { - throw "Left hand side '" + - text.substring(0, token.index) + "' of assignment '" + - text.substring(token.index) + "' is not assignable."; + throwError("implies assignment but [" + + text.substring(0, token.index) + "] can not be assigned to", token); } var ident = function(){return left.isAssignable;}; return binaryFn(ident, token.fn, logicalOR()); @@ -498,8 +478,7 @@ function parser(text, json){ instance = instance[key]; } if (typeof instance != $function) { - throw "Function '" + token.text + "' at column '" + - (token.index+1) + "' in '" + text + "' is not defined."; + throwError("should be a function", token); } return instance; } @@ -518,7 +497,7 @@ function parser(text, json){ var token = expect(); primary = token.fn; if (!primary) { - error("not a primary expression", token); + throwError("not a primary expression", token); } } var next; @@ -530,7 +509,7 @@ function parser(text, json){ } else if (next.text === '.') { primary = fieldAccess(primary); } else { - throw "IMPOSSIBLE"; + throwError("IMPOSSIBLE"); } } return primary; diff --git a/src/services.js b/src/services.js index be442c20..5dfc64f7 100644 --- a/src/services.js +++ b/src/services.js @@ -311,8 +311,6 @@ angularServiceInject("$location", function(browser) { */ angularServiceInject("$log", function($window){ - var console = $window.console || {log: noop, warn: noop, info: noop, error: noop}, - log = console.log || noop; return { /** * @ngdoc method @@ -322,7 +320,7 @@ angularServiceInject("$log", function($window){ * @description * Write a log message */ - log: bind(console, log), + log: consoleLog('log'), /** * @ngdoc method @@ -332,7 +330,7 @@ angularServiceInject("$log", function($window){ * @description * Write a warning message */ - warn: bind(console, console.warn || log), + warn: consoleLog('warn'), /** * @ngdoc method @@ -342,7 +340,7 @@ angularServiceInject("$log", function($window){ * @description * Write an information message */ - info: bind(console, console.info || log), + info: consoleLog('info'), /** * @ngdoc method @@ -352,8 +350,25 @@ angularServiceInject("$log", function($window){ * @description * Write an error message */ - error: bind(console, console.error || log) + error: consoleLog('error') }; + + function consoleLog(type) { + var console = $window.console || {}; + var logFn = console[type] || console.log || noop; + if (logFn.apply) { + return function(){ + var args = []; + foreach(arguments, function(arg){ + args.push(formatError(arg)); + }); + return logFn.apply(console, args); + }; + } else { + // we are IE, in which case there is nothing we can do + return logFn; + } + } }, ['$window'], EAGER_PUBLISHED); /** diff --git a/test/BinderTest.js b/test/BinderTest.js index 71b2f6b6..d35d46f4 100644 --- a/test/BinderTest.js +++ b/test/BinderTest.js @@ -276,15 +276,15 @@ BinderTest.prototype.testIfTextBindingThrowsErrorDecorateTheSpan = function(){ a.scope.$eval(); var span = childNode(doc, 0); assertTrue(span.hasClass('ng-exception')); - assertEquals('ErrorMsg1', fromJson(span.text())); - assertEquals('"ErrorMsg1"', span.attr('ng-exception')); + assertTrue(!!span.text().match(/ErrorMsg1/)); + assertTrue(!!span.attr('ng-exception').match(/ErrorMsg1/)); a.scope.$set('error.throw', function(){throw "MyError";}); a.scope.$eval(); span = childNode(doc, 0); assertTrue(span.hasClass('ng-exception')); assertTrue(span.text(), span.text().match('MyError') !== null); - assertEquals('"MyError"', span.attr('ng-exception')); + assertEquals('MyError', span.attr('ng-exception')); a.scope.$set('error.throw', function(){return "ok";}); a.scope.$eval(); @@ -438,13 +438,12 @@ BinderTest.prototype.testActionOnAHrefThrowsError = function(){ var model = {books:[]}; var c = this.compile('Add Phone', model); c.scope.action = function(){ - throw {a:'abc', b:2}; + throw new Error('MyError'); }; var input = c.node; browserTrigger(input, 'click'); - var error = fromJson(input.attr('ng-exception')); - assertEquals("abc", error.a); - assertEquals(2, error.b); + var error = input.attr('ng-exception'); + assertTrue(!!error.match(/MyError/)); assertTrue("should have an error class", input.hasClass('ng-exception')); // TODO: I think that exception should never get cleared so this portion of test makes no sense diff --git a/test/JsonSpec.js b/test/JsonSpec.js index 0acbd79c..f0019bef 100644 --- a/test/JsonSpec.js +++ b/test/JsonSpec.js @@ -74,7 +74,7 @@ describe('json', function(){ }); it('should serialize $ properties', function() { - var obj = {$a: 'a'} + var obj = {$a: 'a'}; expect(angular.toJson(obj)).toEqual('{"$a":"a"}'); }); @@ -118,31 +118,38 @@ describe('json', function(){ describe('security', function(){ it('should not allow naked expressions', function(){ - expect(function(){fromJson('1+2');}).toThrow("Did not understand '+2' while evaluating '1+2'."); + expect(function(){fromJson('1+2');}). + toThrow(new Error("Parse Error: Token '+' is extra token not part of expression at column 2 of expression [1+2] starting at [+2].")); }); it('should not allow naked expressions group', function(){ - expect(function(){fromJson('(1+2)');}).toThrow("Expression at column='0' of expression '(1+2)' starting at '(1+2)' is not valid json."); + expect(function(){fromJson('(1+2)');}). + toThrow(new Error("Parse Error: Token '(' is not valid json at column 1 of expression [(1+2)] starting at [(1+2)].")); }); it('should not allow expressions in objects', function(){ - expect(function(){fromJson('{a:abc()}');}).toThrow("Expression at column='3' of expression '{a:abc()}' starting at 'abc()}' is not valid json."); + expect(function(){fromJson('{a:abc()}');}). + toThrow(new Error("Parse Error: Token 'abc' is not valid json at column 4 of expression [{a:abc()}] starting at [abc()}].")); }); it('should not allow expressions in arrays', function(){ - expect(function(){fromJson('[1+2]');}).toThrow("Expression at column='2' of expression '[1+2]' starting at '+2]' is not valid json."); + expect(function(){fromJson('[1+2]');}). + toThrow(new Error("Parse Error: Token '+' is not valid json at column 3 of expression [[1+2]] starting at [+2]].")); }); it('should not allow vars', function(){ - expect(function(){fromJson('[1, x]');}).toThrow("Expression at column='4' of expression '[1, x]' starting at 'x]' is not valid json."); + expect(function(){fromJson('[1, x]');}). + toThrow(new Error("Parse Error: Token 'x' is not valid json at column 5 of expression [[1, x]] starting at [x]].")); }); it('should not allow dereference', function(){ - expect(function(){fromJson('["".constructor]');}).toThrow("Expression at column='3' of expression '[\"\".constructor]' starting at '.constructor]' is not valid json."); + expect(function(){fromJson('["".constructor]');}). + toThrow(new Error("Parse Error: Token '.' is not valid json at column 4 of expression [[\"\".constructor]] starting at [.constructor]].")); }); it('should not allow expressions ofter valid json', function(){ - expect(function(){fromJson('[].constructor');}).toThrow("Expression at column='2' of expression '[].constructor' starting at '.constructor' is not valid json."); + expect(function(){fromJson('[].constructor');}). + toThrow(new Error("Parse Error: Token '.' is not valid json at column 3 of expression [[].constructor] starting at [.constructor].")); }); }); diff --git a/test/ParserSpec.js b/test/ParserSpec.js index f772e4a4..06cec2b3 100644 --- a/test/ParserSpec.js +++ b/test/ParserSpec.js @@ -158,11 +158,11 @@ describe('parser', function() { it('should throws exception for invalid exponent', function() { expect(function() { lex("0.5E-"); - }).toThrow('Lexer found invalid exponential value "0.5E-"'); + }).toThrow(new Error('Lexer Error: Invalid exponent at column 4 in expression [0.5E-].')); expect(function() { lex("0.5E-A"); - }).toThrow('Lexer found invalid exponential value "0.5E-A"'); + }).toThrow(new Error('Lexer Error: Invalid exponent at column 4 in expression [0.5E-A].')); }); it('should tokenize number starting with a dot', function() { @@ -173,7 +173,7 @@ describe('parser', function() { it('should throw error on invalid unicode', function() { expect(function() { lex("'\\u1''bla'"); - }).toThrow("Lexer Error: Invalid unicode escape [\\u1''b] starting at column '0' in expression ''\\u1''bla''."); + }).toThrow(new Error("Lexer Error: Invalid unicode escape [\\u1''b] at column 2 in expression ['\\u1''bla'].")); }); }); @@ -225,7 +225,7 @@ describe('parser', function() { expect(function() { scope.$eval("1|nonExistant"); - }).toThrow("Function 'nonExistant' at column '3' in '1|nonExistant' is not defined."); + }).toThrow(new Error("Parse Error: Token 'nonExistant' should be a function at column 3 of expression [1|nonExistant] starting at [nonExistant].")); scope.$set('offset', 3); expect(scope.$eval("'abcd'|upper._case")).toEqual("ABCD"); @@ -312,14 +312,6 @@ describe('parser', function() { expect(scope.$eval(";;1;;")).toEqual(1); }); - it('should evaluate throw', function() { - scope.$set('e', 'abc'); - - expect(function() { - scope.$eval("throw e"); - }).toThrow('abc'); - }); - it('should evaluate object methods in correct context (this)', function() { var C = function () { this.a = 123; diff --git a/test/ScopeSpec.js b/test/ScopeSpec.js index 8b4ada5a..38350b17 100644 --- a/test/ScopeSpec.js +++ b/test/ScopeSpec.js @@ -106,7 +106,7 @@ describe('scope/model', function(){ model.$watch('name', function(newVal, oldVal){ count ++; nameNewVal = newVal; - nameOldVal = oldVal + nameOldVal = oldVal; }); expect(count).toBe(1); @@ -123,12 +123,12 @@ describe('scope/model', function(){ model.$watch('name', function(newVal, oldVal){ count ++; nameNewVal = newVal; - nameOldVal = oldVal + nameOldVal = oldVal; }, undefined, false); expect(count).toBe(0); expect(nameNewVal).toBe('crazy val 1'); - expect(nameOldVal).toBe('crazy val 2') + expect(nameOldVal).toBe('crazy val 2'); }); }); @@ -143,17 +143,17 @@ describe('scope/model', function(){ describe('$tryEval', function(){ it('should report error on element', function(){ var scope = createScope(); - scope.$tryEval('throw "myerror";', function(error){ + scope.$tryEval(function(){throw "myError";}, function(error){ scope.error = error; }); - expect(scope.error).toEqual('myerror'); + expect(scope.error).toEqual('myError'); }); it('should report error on visible element', function(){ var element = jqLite('
'); var scope = createScope(); - scope.$tryEval('throw "myError"', element); - expect(element.attr('ng-exception')).toEqual('"myError"'); // errors are jsonified + scope.$tryEval(function(){throw "myError";}, element); + expect(element.attr('ng-exception')).toEqual('myError'); expect(element.hasClass('ng-exception')).toBeTruthy(); }); @@ -163,7 +163,7 @@ describe('scope/model', function(){ scope.$exceptionHandler = function(e){ this.error = e; }; - scope.$tryEval('throw "myError"'); + scope.$tryEval(function(){throw "myError";}); expect(scope.error).toEqual("myError"); }); }); diff --git a/test/directivesSpec.js b/test/directivesSpec.js index d575c062..4b949fcb 100644 --- a/test/directivesSpec.js +++ b/test/directivesSpec.js @@ -168,7 +168,7 @@ describe("directives", function(){ var log = ""; log += element.attr('ng-exception') + ';'; log += element.hasClass('ng-exception') + ';'; - expect(log).toEqual("\"Expected ng:repeat in form of 'item in collection' but got 'i dont parse'.\";true;"); + expect(log.match(/Expected ng:repeat in form of 'item in collection' but got 'i dont parse'./)).toBeTruthy(); }); it('should expose iterator offset as $index when iterating over arrays', function() { diff --git a/test/servicesSpec.js b/test/servicesSpec.js index f4d9aabe..13e61b18 100644 --- a/test/servicesSpec.js +++ b/test/servicesSpec.js @@ -70,6 +70,42 @@ describe("service", function(){ scope.$log.info(); scope.$log.error(); }); + + describe('Error', function(){ + var e, $log, $console, errorArgs; + beforeEach(function(){ + e = new Error(''); + e.message = undefined; + e.sourceURL = undefined; + e.line = undefined; + e.stack = undefined; + + $console = angular.service('$log')({console:{error:function(){ + errorArgs = arguments; + }}}); + }); + + it('should pass error if does not have trace', function(){ + $console.error('abc', e); + expect(errorArgs).toEqual(['abc', e]); + }); + + it('should print stack', function(){ + e.stack = 'stack'; + $console.error('abc', e); + expect(errorArgs).toEqual(['abc', 'stack']); + }); + + it('should print line', function(){ + e.message = 'message'; + e.sourceURL = 'sourceURL'; + e.line = '123'; + $console.error('abc', e); + expect(errorArgs).toEqual(['abc', 'message\nsourceURL:123']); + }); + + }); + }); describe("$exceptionHandler", function(){ -- cgit v1.2.3