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

Commit 3dc0096

Browse files
committed
fix($log): don't parse error stacks manually outside of IE/Edge
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. Fixes #15590 Closes #15767
1 parent 846fa1c commit 3dc0096

File tree

2 files changed

+49
-24
lines changed

2 files changed

+49
-24
lines changed

src/ng/log.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,15 @@ function $LogProvider() {
6767
};
6868

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

125134
function formatError(arg) {
126135
if (arg instanceof Error) {
127-
if (arg.stack) {
136+
if (arg.stack && formatStackTrace) {
128137
arg = (arg.message && arg.stack.indexOf(arg.message) === -1)
129138
? 'Error: ' + arg.message + '\n' + arg.stack
130139
: arg.stack;

test/ng/logSpec.js

+39-23
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ describe('$log', function() {
77

88

99
beforeEach(module(function($provide) {
10-
$window = {navigator: {}, document: {}};
10+
$window = {
11+
navigator: {userAgent: window.navigator.userAgent},
12+
document: {}
13+
};
1114
logger = '';
1215
log = function() { logger += 'log;'; };
1316
warn = function() { logger += 'warn;'; };
@@ -64,6 +67,13 @@ describe('$log', function() {
6467
}
6568
));
6669

70+
it('should work if $window.navigator not defined', inject(
71+
function() {
72+
delete $window.navigator;
73+
},
74+
function($log) {}
75+
));
76+
6777
describe('IE logging behavior', function() {
6878
function removeApplyFunctionForIE() {
6979
log.apply = log.call =
@@ -131,12 +141,12 @@ describe('$log', function() {
131141
$log.debug();
132142
expect(logger).toEqual('log;warn;info;error;');
133143
}
134-
));
144+
));
135145

136146
});
137147

138148
describe('$log.error', function() {
139-
var e, $log, errorArgs;
149+
var e, $log;
140150

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

151-
beforeEach(function() {
152-
e = new TestError('');
153-
var mockWindow = {
154-
console: {
155-
error: function() {
156-
errorArgs = [].slice.call(arguments, 0);
157-
}
158-
}
159-
};
160-
$log = new $LogProvider().$get[1](mockWindow);
161-
});
161+
beforeEach(inject(
162+
function() {
163+
e = new TestError('');
164+
$window.console = {
165+
error: jasmine.createSpy('error')
166+
};
167+
},
162168

169+
function(_$log_) {
170+
$log = _$log_;
171+
}
172+
));
163173

164174
it('should pass error if does not have trace', function() {
165175
$log.error('abc', e);
166-
expect(errorArgs).toEqual(['abc', e]);
167-
});
168-
169-
170-
it('should print stack', function() {
171-
e.stack = 'stack';
172-
$log.error('abc', e);
173-
expect(errorArgs).toEqual(['abc', 'stack']);
176+
expect($window.console.error).toHaveBeenCalledWith('abc', e);
174177
});
175178

179+
if (msie || /\bEdge\//.test(window.navigator.userAgent)) {
180+
it('should print stack', function() {
181+
e.stack = 'stack';
182+
$log.error('abc', e);
183+
expect($window.console.error).toHaveBeenCalledWith('abc', 'stack');
184+
});
185+
} else {
186+
it('should print a raw error', function() {
187+
e.stack = 'stack';
188+
$log.error('abc', e);
189+
expect($window.console.error).toHaveBeenCalledWith('abc', e);
190+
});
191+
}
176192

177193
it('should print line', function() {
178194
e.message = 'message';
179195
e.sourceURL = 'sourceURL';
180196
e.line = '123';
181197
$log.error('abc', e);
182-
expect(errorArgs).toEqual(['abc', 'message\nsourceURL:123']);
198+
expect($window.console.error).toHaveBeenCalledWith('abc', 'message\nsourceURL:123');
183199
});
184200
});
185201

0 commit comments

Comments
 (0)