From dd8a65cb603b193925d99cad606656878e079594 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sat, 14 Aug 2021 14:01:15 -0700 Subject: [PATCH] Better reporting of unhandled promise rejections with truthy but non-`Error` reasons on Node [#179227413] --- lib/jasmine-core/jasmine.js | 12 ++- spec/core/GlobalErrorsSpec.js | 170 ++++++++++++++++++++-------------- src/core/GlobalErrors.js | 12 ++- 3 files changed, 120 insertions(+), 74 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 4502009a..a80df7b1 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -4333,16 +4333,22 @@ getJasmineRequireObj().GlobalErrors = function(j$) { function taggedOnError(error) { var substituteMsg; - if (error) { + if (j$.isError_(error)) { error.jasmineMessage = jasmineMessage + ': ' + error; } else { - substituteMsg = jasmineMessage + ' with no error or message'; + if (error) { + substituteMsg = jasmineMessage + ': ' + error; + } else { + substituteMsg = jasmineMessage + ' with no error or message'; + } if (errorType === 'unhandledRejection') { substituteMsg += '\n' + '(Tip: to get a useful stack trace, use ' + - 'Promise.reject(new Error(...)) instead of Promise.reject().)'; + 'Promise.reject(new Error(...)) instead of Promise.reject(' + + (error ? '...' : '') + + ').)'; } error = new Error(substituteMsg); diff --git a/spec/core/GlobalErrorsSpec.js b/spec/core/GlobalErrorsSpec.js index 8e3ffbc5..07e2f48c 100644 --- a/spec/core/GlobalErrorsSpec.js +++ b/spec/core/GlobalErrorsSpec.js @@ -170,84 +170,118 @@ describe('GlobalErrors', function() { ); }); - it('reports unhandled promise rejections in node.js', function() { - var fakeGlobal = { - process: { - on: jasmine.createSpy('process.on'), - removeListener: jasmine.createSpy('process.removeListener'), - listeners: jasmine - .createSpy('process.listeners') - .and.returnValue(['foo']), - removeAllListeners: jasmine.createSpy('process.removeAllListeners') - } - }, - handler = jasmine.createSpy('errorHandler'), - errors = new jasmineUnderTest.GlobalErrors(fakeGlobal); + describe('Reporting unhandled promise rejections in node.js', function() { + it('reports rejections with `Error` reasons', function() { + var fakeGlobal = { + process: { + on: jasmine.createSpy('process.on'), + removeListener: jasmine.createSpy('process.removeListener'), + listeners: jasmine + .createSpy('process.listeners') + .and.returnValue(['foo']), + removeAllListeners: jasmine.createSpy('process.removeAllListeners') + } + }, + handler = jasmine.createSpy('errorHandler'), + errors = new jasmineUnderTest.GlobalErrors(fakeGlobal); - errors.install(); - expect(fakeGlobal.process.on).toHaveBeenCalledWith( - 'unhandledRejection', - jasmine.any(Function) - ); - expect(fakeGlobal.process.listeners).toHaveBeenCalledWith( - 'unhandledRejection' - ); - expect(fakeGlobal.process.removeAllListeners).toHaveBeenCalledWith( - 'unhandledRejection' - ); + errors.install(); + expect(fakeGlobal.process.on).toHaveBeenCalledWith( + 'unhandledRejection', + jasmine.any(Function) + ); + expect(fakeGlobal.process.listeners).toHaveBeenCalledWith( + 'unhandledRejection' + ); + expect(fakeGlobal.process.removeAllListeners).toHaveBeenCalledWith( + 'unhandledRejection' + ); - errors.pushListener(handler); + errors.pushListener(handler); - var addedListener = fakeGlobal.process.on.calls.argsFor(1)[1]; - addedListener(new Error('bar')); + var addedListener = fakeGlobal.process.on.calls.argsFor(1)[1]; + addedListener(new Error('bar')); - expect(handler).toHaveBeenCalledWith(new Error('bar')); - expect(handler.calls.argsFor(0)[0].jasmineMessage).toBe( - 'Unhandled promise rejection: Error: bar' - ); + expect(handler).toHaveBeenCalledWith(new Error('bar')); + expect(handler.calls.argsFor(0)[0].jasmineMessage).toBe( + 'Unhandled promise rejection: Error: bar' + ); - errors.uninstall(); + errors.uninstall(); - expect(fakeGlobal.process.removeListener).toHaveBeenCalledWith( - 'unhandledRejection', - addedListener - ); - expect(fakeGlobal.process.on).toHaveBeenCalledWith( - 'unhandledRejection', - 'foo' - ); - }); + expect(fakeGlobal.process.removeListener).toHaveBeenCalledWith( + 'unhandledRejection', + addedListener + ); + expect(fakeGlobal.process.on).toHaveBeenCalledWith( + 'unhandledRejection', + 'foo' + ); + }); - it('reports unhandled promise rejections in node.js when no error is provided', function() { - var fakeGlobal = { - process: { - on: jasmine.createSpy('process.on'), - removeListener: function() {}, - listeners: function() { - return []; - }, - removeAllListeners: function() {} - } - }, - handler = jasmine.createSpy('errorHandler'), - errors = new jasmineUnderTest.GlobalErrors(fakeGlobal); + it('reports rejections with non-`Error` reasons', function() { + var fakeGlobal = { + process: { + on: jasmine.createSpy('process.on'), + removeListener: function() {}, + listeners: function() { + return []; + }, + removeAllListeners: function() {} + } + }, + handler = jasmine.createSpy('errorHandler'), + errors = new jasmineUnderTest.GlobalErrors(fakeGlobal); - errors.install(); - errors.pushListener(handler); + errors.install(); + errors.pushListener(handler); - expect(fakeGlobal.process.on.calls.argsFor(1)[0]).toEqual( - 'unhandledRejection' - ); - var addedListener = fakeGlobal.process.on.calls.argsFor(1)[1]; - addedListener(undefined); + expect(fakeGlobal.process.on.calls.argsFor(1)[0]).toEqual( + 'unhandledRejection' + ); + var addedListener = fakeGlobal.process.on.calls.argsFor(1)[1]; + addedListener(17); - expect(handler).toHaveBeenCalledWith( - new Error( - 'Unhandled promise rejection with no error or message\n' + - '(Tip: to get a useful stack trace, use ' + - 'Promise.reject(new Error(...)) instead of Promise.reject().)' - ) - ); + expect(handler).toHaveBeenCalledWith( + new Error( + 'Unhandled promise rejection: 17\n' + + '(Tip: to get a useful stack trace, use ' + + 'Promise.reject(new Error(...)) instead of Promise.reject(...).)' + ) + ); + }); + + it('reports rejections with no reason provided', function() { + var fakeGlobal = { + process: { + on: jasmine.createSpy('process.on'), + removeListener: function() {}, + listeners: function() { + return []; + }, + removeAllListeners: function() {} + } + }, + handler = jasmine.createSpy('errorHandler'), + errors = new jasmineUnderTest.GlobalErrors(fakeGlobal); + + errors.install(); + errors.pushListener(handler); + + expect(fakeGlobal.process.on.calls.argsFor(1)[0]).toEqual( + 'unhandledRejection' + ); + var addedListener = fakeGlobal.process.on.calls.argsFor(1)[1]; + addedListener(undefined); + + expect(handler).toHaveBeenCalledWith( + new Error( + 'Unhandled promise rejection with no error or message\n' + + '(Tip: to get a useful stack trace, use ' + + 'Promise.reject(new Error(...)) instead of Promise.reject().)' + ) + ); + }); }); describe('Reporting unhandled promise rejections in the browser', function() { diff --git a/src/core/GlobalErrors.js b/src/core/GlobalErrors.js index b0c6e20e..d96e26c8 100644 --- a/src/core/GlobalErrors.js +++ b/src/core/GlobalErrors.js @@ -19,16 +19,22 @@ getJasmineRequireObj().GlobalErrors = function(j$) { function taggedOnError(error) { var substituteMsg; - if (error) { + if (j$.isError_(error)) { error.jasmineMessage = jasmineMessage + ': ' + error; } else { - substituteMsg = jasmineMessage + ' with no error or message'; + if (error) { + substituteMsg = jasmineMessage + ': ' + error; + } else { + substituteMsg = jasmineMessage + ' with no error or message'; + } if (errorType === 'unhandledRejection') { substituteMsg += '\n' + '(Tip: to get a useful stack trace, use ' + - 'Promise.reject(new Error(...)) instead of Promise.reject().)'; + 'Promise.reject(new Error(...)) instead of Promise.reject(' + + (error ? '...' : '') + + ').)'; } error = new Error(substituteMsg);