From df4b6e58e2063b6bccb762927dcc1bb6c45376d4 Mon Sep 17 00:00:00 2001 From: Ferdinand Prantl Date: Mon, 15 Jul 2019 14:43:22 +0200 Subject: [PATCH 1/5] Pass the error including stacktrace to error handlers and reporters The global window error handler is used to handle errors thrown from within asynchronous functions and tests. The first parameter is the error; the fifth parameter is the full error object including the stacktrace. Searching for the first occurrence of an error instance to work with browsers, which may not comply with the HTML5 standard. --- src/core/GlobalErrors.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/GlobalErrors.js b/src/core/GlobalErrors.js index 2c58e684..e8430c1e 100644 --- a/src/core/GlobalErrors.js +++ b/src/core/GlobalErrors.js @@ -7,7 +7,12 @@ getJasmineRequireObj().GlobalErrors = function(j$) { var handler = handlers[handlers.length - 1]; if (handler) { - handler.apply(null, Array.prototype.slice.call(arguments, 0)); + // Get error from (message, source, lineno, colno, error) + var args = Array.prototype.slice.call(arguments, 0); + var error = args.find(function(arg) { + return arg instanceof Error; + }); + handler.apply(null, error ? [error] : args); } else { throw arguments[0]; } From 4858a62fdc55516888c906cb91dc3c37925c44c6 Mon Sep 17 00:00:00 2001 From: Ferdinand Prantl Date: Tue, 16 Jul 2019 12:00:09 +0200 Subject: [PATCH 2/5] Add a unit test for the global error handling including stacktrace --- spec/core/GlobalErrorsSpec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/core/GlobalErrorsSpec.js b/spec/core/GlobalErrorsSpec.js index 9ff19891..b4a54282 100644 --- a/spec/core/GlobalErrorsSpec.js +++ b/spec/core/GlobalErrorsSpec.js @@ -12,6 +12,20 @@ describe('GlobalErrors', function() { expect(handler).toHaveBeenCalledWith('foo'); }); + it('prefers passing the error including stack to the handler', function() { + var fakeGlobal = { onerror: null }, + handler = jasmine.createSpy('errorHandler'), + errors = new jasmineUnderTest.GlobalErrors(fakeGlobal), + fooError = new Error('foo'); + + errors.install(); + errors.pushListener(handler); + + fakeGlobal.onerror(fooError.message, 'foo.js', 1, 1, fooError); + + expect(handler).toHaveBeenCalledWith(fooError); + }); + it('only calls the most recent handler', function() { var fakeGlobal = { onerror: null }, handler1 = jasmine.createSpy('errorHandler1'), From 7c3434723e05fad11ad5d6e6a532944e88c2fb42 Mon Sep 17 00:00:00 2001 From: Ferdinand Prantl Date: Sun, 21 Jul 2019 23:46:14 +0200 Subject: [PATCH 3/5] Use the documented interface to pick the error instance from the global error handler --- src/core/GlobalErrors.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/core/GlobalErrors.js b/src/core/GlobalErrors.js index e8430c1e..f62208b4 100644 --- a/src/core/GlobalErrors.js +++ b/src/core/GlobalErrors.js @@ -3,16 +3,14 @@ getJasmineRequireObj().GlobalErrors = function(j$) { var handlers = []; global = global || j$.getGlobal(); - var onerror = function onerror() { + var onerror = function onerror(message, source, lineno, colno, error) { var handler = handlers[handlers.length - 1]; if (handler) { - // Get error from (message, source, lineno, colno, error) var args = Array.prototype.slice.call(arguments, 0); - var error = args.find(function(arg) { - return arg instanceof Error; - }); - handler.apply(null, error ? [error] : args); + // Prefer passing the error to the error handler + // to be able to print the stack trace. + handler.apply(null, error instanceof Error ? [error] : args); } else { throw arguments[0]; } From 527619b0aa5cdfc7bc5f8f863680a2df8bc4f58e Mon Sep 17 00:00:00 2001 From: Ferdinand Prantl Date: Sun, 11 Aug 2019 09:31:43 +0200 Subject: [PATCH 4/5] Restore the original global error hanler to pass all parameters along --- spec/core/GlobalErrorsSpec.js | 10 ++++++++-- src/core/GlobalErrors.js | 7 ++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/spec/core/GlobalErrorsSpec.js b/spec/core/GlobalErrorsSpec.js index b4a54282..3880b041 100644 --- a/spec/core/GlobalErrorsSpec.js +++ b/spec/core/GlobalErrorsSpec.js @@ -12,7 +12,7 @@ describe('GlobalErrors', function() { expect(handler).toHaveBeenCalledWith('foo'); }); - it('prefers passing the error including stack to the handler', function() { + it('calls the global error handler with all parameters', function() { var fakeGlobal = { onerror: null }, handler = jasmine.createSpy('errorHandler'), errors = new jasmineUnderTest.GlobalErrors(fakeGlobal), @@ -23,7 +23,13 @@ describe('GlobalErrors', function() { fakeGlobal.onerror(fooError.message, 'foo.js', 1, 1, fooError); - expect(handler).toHaveBeenCalledWith(fooError); + expect(handler).toHaveBeenCalledWith( + fooError.message, + 'foo.js', + 1, + 1, + fooError + ); }); it('only calls the most recent handler', function() { diff --git a/src/core/GlobalErrors.js b/src/core/GlobalErrors.js index f62208b4..2c58e684 100644 --- a/src/core/GlobalErrors.js +++ b/src/core/GlobalErrors.js @@ -3,14 +3,11 @@ getJasmineRequireObj().GlobalErrors = function(j$) { var handlers = []; global = global || j$.getGlobal(); - var onerror = function onerror(message, source, lineno, colno, error) { + var onerror = function onerror() { var handler = handlers[handlers.length - 1]; if (handler) { - var args = Array.prototype.slice.call(arguments, 0); - // Prefer passing the error to the error handler - // to be able to print the stack trace. - handler.apply(null, error instanceof Error ? [error] : args); + handler.apply(null, Array.prototype.slice.call(arguments, 0)); } else { throw arguments[0]; } From 3a7fc638790014d6ac79b0bdac88976655ba5dc4 Mon Sep 17 00:00:00 2001 From: Ferdinand Prantl Date: Sun, 11 Aug 2019 09:32:31 +0200 Subject: [PATCH 5/5] Pick the error instance to pass to error handlers in QueueRunner The first number is the error message in HTML5 browser, which does not include the call stack. The error instance allows logging the complete call stack in reporters. --- spec/core/QueueRunnerSpec.js | 26 ++++++++++++++++++++++++++ src/core/QueueRunner.js | 7 +++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index e0b4c23c..ac18b81d 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -514,6 +514,32 @@ describe('QueueRunner', function() { }); }); + it('passes the error instance to exception handlers in HTML browsers', function() { + var error = new Error('fake error'), + onExceptionCallback = jasmine.createSpy('on exception callback'), + queueRunner = new jasmineUnderTest.QueueRunner({ + onException: onExceptionCallback + }); + + queueRunner.execute(); + queueRunner.handleFinalError(error.message, 'fake.js', 1, 1, error); + + expect(onExceptionCallback).toHaveBeenCalledWith(error); + }); + + it('passes the first argument to exception handlers for compatibility', function() { + var error = new Error('fake error'), + onExceptionCallback = jasmine.createSpy('on exception callback'), + queueRunner = new jasmineUnderTest.QueueRunner({ + onException: onExceptionCallback + }); + + queueRunner.execute(); + queueRunner.handleFinalError(error.message); + + expect(onExceptionCallback).toHaveBeenCalledWith(error.message); + }); + it('calls exception handlers when an exception is thrown in a fn', function() { var queueableFn = { type: 'queueable', diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index 27b57695..639296d7 100644 --- a/src/core/QueueRunner.js +++ b/src/core/QueueRunner.js @@ -49,8 +49,11 @@ getJasmineRequireObj().QueueRunner = function(j$) { QueueRunner.prototype.execute = function() { var self = this; - this.handleFinalError = function(error) { - self.onException(error); + this.handleFinalError = function(message, source, lineno, colno, error) { + // Older browsers would send the error as the first parameter. HTML5 + // specifies the the five parameters above. The error instance should + // be preffered, otherwise the call stack would get lost. + self.onException(error || message); }; this.globalErrors.pushListener(this.handleFinalError); this.run(0);