Skip to content

Commit c14c874

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 c14c874

File tree

2 files changed

+83
-33
lines changed

2 files changed

+83
-33
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

+75-32
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ describe('$log', function() {
77

88

99
beforeEach(module(function($provide) {
10-
$window = {navigator: {}, document: {}};
10+
$window = {navigator: {userAgent: ''}, document: {}};
1111
logger = '';
1212
log = function() { logger += 'log;'; };
1313
warn = function() { logger += 'warn;'; };
@@ -131,12 +131,12 @@ describe('$log', function() {
131131
$log.debug();
132132
expect(logger).toEqual('log;warn;info;error;');
133133
}
134-
));
134+
));
135135

136136
});
137137

138138
describe('$log.error', function() {
139-
var e, $log, errorArgs;
139+
var e, $log;
140140

141141
function TestError() {
142142
Error.prototype.constructor.apply(this, arguments);
@@ -148,38 +148,81 @@ describe('$log', function() {
148148
TestError.prototype = Object.create(Error.prototype);
149149
TestError.prototype.constructor = TestError;
150150

151-
beforeEach(function() {
152-
e = new TestError('');
153-
var mockWindow = {
154-
console: {
155-
error: function() {
156-
errorArgs = [].slice.call(arguments, 0);
151+
[
152+
{
153+
browser: 'Chrome',
154+
msie: undefined,
155+
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 ' +
156+
'(KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36'
157+
},
158+
{
159+
browser: 'Firefox',
160+
msie: undefined,
161+
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:51.0) ' +
162+
'Gecko/20100101 Firefox/51.0'
163+
},
164+
{
165+
browser: 'Safari',
166+
msie: undefined,
167+
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/602.4.8 ' +
168+
'(KHTML, like Gecko) Version/10.0.3 Safari/602.4.8'
169+
},
170+
{
171+
browser: 'Edge',
172+
msie: undefined,
173+
userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 ' +
174+
'(KHTML, like Gecko) Chrome/51.0.2704.79 Safari/537.36 Edge/14.14393'
175+
},
176+
{
177+
browser: 'IE',
178+
msie: 11,
179+
userAgent: 'Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; .NET4.0C; .NET4.0E; rv:11.0) like Gecko'
180+
}
181+
].forEach(function(options) {
182+
describe('(in ' + options.browser + ')', function() {
183+
beforeEach(inject(
184+
function() {
185+
e = new TestError('');
186+
$window.console = {
187+
error: jasmine.createSpy('error')
188+
};
189+
$window.navigator.userAgent = options.userAgent;
190+
msie = options.msie;
191+
},
192+
193+
function(_$log_) {
194+
$log = _$log_;
157195
}
196+
));
197+
198+
199+
it('should pass error if does not have trace', function() {
200+
$log.error('abc', e);
201+
expect($window.console.error).toHaveBeenCalledWith('abc', e);
202+
});
203+
204+
if (options.browser === 'IE' || options.browser === 'Edge') {
205+
it('should print stack', function() {
206+
e.stack = 'stack';
207+
$log.error('abc', e);
208+
expect($window.console.error).toHaveBeenCalledWith('abc', 'stack');
209+
});
210+
} else {
211+
it('should print a raw error', function() {
212+
e.stack = 'stack';
213+
$log.error('abc', e);
214+
expect($window.console.error).toHaveBeenCalledWith('abc', e);
215+
});
158216
}
159-
};
160-
$log = new $LogProvider().$get[1](mockWindow);
161-
});
162-
163-
164-
it('should pass error if does not have trace', function() {
165-
$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']);
174-
});
175-
176217

177-
it('should print line', function() {
178-
e.message = 'message';
179-
e.sourceURL = 'sourceURL';
180-
e.line = '123';
181-
$log.error('abc', e);
182-
expect(errorArgs).toEqual(['abc', 'message\nsourceURL:123']);
218+
it('should print line', function() {
219+
e.message = 'message';
220+
e.sourceURL = 'sourceURL';
221+
e.line = '123';
222+
$log.error('abc', e);
223+
expect($window.console.error).toHaveBeenCalledWith('abc', 'message\nsourceURL:123');
224+
});
225+
});
183226
});
184227
});
185228

0 commit comments

Comments
 (0)