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

fix($log): don't parse error stacks manually outside of IE/Edge #15767

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 10 additions & 1 deletion src/ng/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ function $LogProvider() {
};

this.$get = ['$window', function($window) {
// Support: IE 9-11, Edge 12-14+
// IE/Edge display errors in such a way that it requires the user to click in 4 places
// to see the stack trace. There is no way to feature-detect it so there's a chance
// of the user agent sniffing to go wrong but since it's only about logging, this shouldn't
// break apps. Other browsers display errors in a sensible way and some of them map stack
// traces along source maps if available so it makes sense to let browsers display it
// as they want.
var formatStackTrace = msie || /\bEdge\//.test($window.navigator && $window.navigator.userAgent);

return {
/**
* @ngdoc method
Expand Down Expand Up @@ -124,7 +133,7 @@ function $LogProvider() {

function formatError(arg) {
if (arg instanceof Error) {
if (arg.stack) {
if (arg.stack && formatStackTrace) {
arg = (arg.message && arg.stack.indexOf(arg.message) === -1)
? 'Error: ' + arg.message + '\n' + arg.stack
: arg.stack;
Expand Down
62 changes: 39 additions & 23 deletions test/ng/logSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ describe('$log', function() {


beforeEach(module(function($provide) {
$window = {navigator: {}, document: {}};
$window = {
navigator: {userAgent: window.navigator.userAgent},
document: {}
};
logger = '';
log = function() { logger += 'log;'; };
warn = function() { logger += 'warn;'; };
Expand Down Expand Up @@ -64,6 +67,13 @@ describe('$log', function() {
}
));

it('should work if $window.navigator not defined', inject(
function() {
delete $window.navigator;
},
function($log) {}
));

describe('IE logging behavior', function() {
function removeApplyFunctionForIE() {
log.apply = log.call =
Expand Down Expand Up @@ -131,12 +141,12 @@ describe('$log', function() {
$log.debug();
expect(logger).toEqual('log;warn;info;error;');
}
));
));

});

describe('$log.error', function() {
var e, $log, errorArgs;
var e, $log;

function TestError() {
Error.prototype.constructor.apply(this, arguments);
Expand All @@ -148,38 +158,44 @@ describe('$log', function() {
TestError.prototype = Object.create(Error.prototype);
TestError.prototype.constructor = TestError;

beforeEach(function() {
e = new TestError('');
var mockWindow = {
console: {
error: function() {
errorArgs = [].slice.call(arguments, 0);
}
}
};
$log = new $LogProvider().$get[1](mockWindow);
});
beforeEach(inject(
function() {
e = new TestError('');
$window.console = {
error: jasmine.createSpy('error')
};
},

function(_$log_) {
$log = _$log_;
}
));

it('should pass error if does not have trace', function() {
$log.error('abc', e);
expect(errorArgs).toEqual(['abc', e]);
});


it('should print stack', function() {
e.stack = 'stack';
$log.error('abc', e);
expect(errorArgs).toEqual(['abc', 'stack']);
expect($window.console.error).toHaveBeenCalledWith('abc', e);
});

if (msie || /\bEdge\//.test(window.navigator.userAgent)) {
it('should print stack', function() {
e.stack = 'stack';
$log.error('abc', e);
expect($window.console.error).toHaveBeenCalledWith('abc', 'stack');
});
} else {
it('should print a raw error', function() {
e.stack = 'stack';
$log.error('abc', e);
expect($window.console.error).toHaveBeenCalledWith('abc', e);
});
}

it('should print line', function() {
e.message = 'message';
e.sourceURL = 'sourceURL';
e.line = '123';
$log.error('abc', e);
expect(errorArgs).toEqual(['abc', 'message\nsourceURL:123']);
expect($window.console.error).toHaveBeenCalledWith('abc', 'message\nsourceURL:123');
});
});

Expand Down