Skip to content

Commit 4913509

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 angular#15590
1 parent e269c14 commit 4913509

File tree

2 files changed

+40
-24
lines changed

2 files changed

+40
-24
lines changed

src/ng/log.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,14 @@ function $LogProvider() {
124124

125125
function formatError(arg) {
126126
if (arg instanceof Error) {
127-
if (arg.stack) {
127+
// Support: IE 9-11, Edge 12-14+
128+
// IE/Edge display errors in such a way that it requires the user to click in 4 places
129+
// to see the stack trace. There is no way to feature-detect it so there's a chance
130+
// of the user agent sniffing to go wrong but since it's only about logging, this shouldn't
131+
// break apps. Other browsers display errors in a sensible way and some of them map stack
132+
// traces along source maps if available so it makes sense to let browsers display it
133+
// as they want.
134+
if (arg.stack && (msie || /\bEdge\//.test($window.navigator.userAgent))) {
128135
arg = (arg.message && arg.stack.indexOf(arg.message) === -1)
129136
? 'Error: ' + arg.message + '\n' + arg.stack
130137
: arg.stack;

test/ng/logSpec.js

+32-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;'; };
@@ -131,12 +134,12 @@ describe('$log', function() {
131134
$log.debug();
132135
expect(logger).toEqual('log;warn;info;error;');
133136
}
134-
));
137+
));
135138

136139
});
137140

138141
describe('$log.error', function() {
139-
var e, $log, errorArgs;
142+
var e, $log;
140143

141144
function TestError() {
142145
Error.prototype.constructor.apply(this, arguments);
@@ -148,38 +151,44 @@ describe('$log', function() {
148151
TestError.prototype = Object.create(Error.prototype);
149152
TestError.prototype.constructor = TestError;
150153

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-
});
154+
beforeEach(inject(
155+
function() {
156+
e = new TestError('');
157+
$window.console = {
158+
error: jasmine.createSpy('error')
159+
};
160+
},
162161

162+
function(_$log_) {
163+
$log = _$log_;
164+
}
165+
));
163166

164167
it('should pass error if does not have trace', function() {
165168
$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']);
169+
expect($window.console.error).toHaveBeenCalledWith('abc', e);
174170
});
175171

172+
if (msie || /\bEdge\//.test(window.navigator.userAgent)) {
173+
it('should print stack', function() {
174+
e.stack = 'stack';
175+
$log.error('abc', e);
176+
expect($window.console.error).toHaveBeenCalledWith('abc', 'stack');
177+
});
178+
} else {
179+
it('should print a raw error', function() {
180+
e.stack = 'stack';
181+
$log.error('abc', e);
182+
expect($window.console.error).toHaveBeenCalledWith('abc', e);
183+
});
184+
}
176185

177186
it('should print line', function() {
178187
e.message = 'message';
179188
e.sourceURL = 'sourceURL';
180189
e.line = '123';
181190
$log.error('abc', e);
182-
expect(errorArgs).toEqual(['abc', 'message\nsourceURL:123']);
191+
expect($window.console.error).toHaveBeenCalledWith('abc', 'message\nsourceURL:123');
183192
});
184193
});
185194

0 commit comments

Comments
 (0)