Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Naive implementation for better error messages from $interpolate #1187

Closed
wants to merge 3 commits into from
Closed

Naive implementation for better error messages from $interpolate #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5b9ceb
naive implementation for better error messages from
btford Jul 19, 2012
b39ba96
fix(): improved exception handling
btford Aug 14, 2012
9dc6803
test(): test interpolation with exceptions
btford Aug 14, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/ng/interpolate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function $InterpolateProvider() {
};


this.$get = ['$parse', function($parse) {
this.$get = ['$parse', '$exceptionHandler', function($parse, $exceptionHandler) {
var startSymbolLength = startSymbol.length,
endSymbolLength = endSymbol.length;

Expand Down Expand Up @@ -122,18 +122,24 @@ function $InterpolateProvider() {
if (!mustHaveExpression || hasInterpolation) {
concat.length = length;
fn = function(context) {
for(var i = 0, ii = length, part; i<ii; i++) {
if (typeof (part = parts[i]) == 'function') {
part = part(context);
if (part == null || part == undefined) {
part = '';
} else if (typeof part != 'string') {
part = toJson(part);
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This try-catch adds perf impact. Do you have a benchmark? a better way would be to publish handleError method on the $interpolate and then have the $digest see if the getter has handleError if so call it with the exception. This way we are not adding any erros.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I benchmarked this and the difference is unmeasurable. So I think this change should be merged as is.

for(var i = 0, ii = length, part; i<ii; i++) {
if (typeof (part = parts[i]) == 'function') {
part = part(context);
if (part == null || part == undefined) {
part = '';
} else if (typeof part != 'string') {
part = toJson(part);
}
}
concat[i] = part;
}
concat[i] = part;
return concat.join('');
}
catch(err) {
var newErr = new Error('Error while interpolating: ' + text + '\n' + err.toString());
$exceptionHandler(newErr);
}
return concat.join('');
};
fn.exp = text;
fn.parts = parts;
Expand Down
2 changes: 1 addition & 1 deletion test/BinderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ describe('Binder', function() {
$rootScope.error['throw'] = function() {throw 'MyError';};
errorLogs.length = 0;
$rootScope.$apply();
expect(errorLogs.shift()).toBe('MyError');
expect(errorLogs.shift().message).toBe('Error while interpolating: {{error.throw()}}\nMyError');

$rootScope.error['throw'] = function() {return 'ok';};
$rootScope.$apply();
Expand Down
23 changes: 23 additions & 0 deletions test/ng/interpolateSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,29 @@ describe('$interpolate', function() {
expect($interpolate('{{ false }}')()).toEqual('false');
}));

it('should rethrow exceptions', inject(function($interpolate, $rootScope) {
$rootScope.err = function () {
throw new Error('oops');
};
expect(function () {
$interpolate('{{err()}}')($rootScope);
}).toThrow('Error while interpolating: {{err()}}\nError: oops');
}));

iit('should stop interpolation when encountering an exception', inject(function($interpolate, $compile, $rootScope) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iit -> it

$rootScope.err = function () {
throw new Error('oops');
};
var dom = jqLite('<div>{{1 + 1}}</div><div>{{err()}}</div><div>{{1 + 2}}</div>');
$compile(dom)($rootScope);
expect(function () {
$rootScope.$apply();
}).toThrow('Error while interpolating: {{err()}}\nError: oops');
expect(dom[0].innerHTML).toEqual('2');
expect(dom[1].innerHTML).toEqual('{{err()}}');
expect(dom[2].innerHTML).toEqual('{{1 + 2}}');
}));


it('should return interpolation function', inject(function($interpolate, $rootScope) {
$rootScope.name = 'Misko';
Expand Down