From a203e88aa53d55c75f0ce1679b0e9ca5d7337455 Mon Sep 17 00:00:00 2001 From: Gergo Tolnai Date: Thu, 8 Dec 2022 10:52:41 +0100 Subject: [PATCH 1/5] fix: log error on reject with string content --- packages/driver/cypress/component/spec.cy.js | 27 ++++++++++++++++++- .../cypress/e2e/cypress/error_utils.cy.ts | 25 +++++++++++++++-- packages/driver/src/cypress/error_utils.ts | 25 +++++++++++++++-- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/packages/driver/cypress/component/spec.cy.js b/packages/driver/cypress/component/spec.cy.js index 13bca8e4afe3..3300813ac30b 100644 --- a/packages/driver/cypress/component/spec.cy.js +++ b/packages/driver/cypress/component/spec.cy.js @@ -1,3 +1,5 @@ +import sinon from 'sinon' + describe('component testing', () => { /** @type {Cypress.Agent} */ let uncaughtExceptionStub @@ -12,17 +14,40 @@ describe('component testing', () => { }) }) + beforeEach(() => { + uncaughtExceptionStub.resetHistory() + document.querySelector('[data-cy-root]').innerHTML = '' + }) + it('fails and shows an error', () => { + cy.spy(Cypress, 'log').log(false) const $el = document.createElement('button') $el.innerText = `Don't click it!` $el.addEventListener('click', () => { - throw Error('An error!') + throw new Error('An error!') }) document.querySelector('[data-cy-root]').appendChild($el) cy.get('button').click().then(() => { expect(uncaughtExceptionStub).to.have.been.calledOnceWithExactly(null) + expect(Cypress.log).to.be.calledWithMatch(sinon.match({ 'message': `Error: An error!`, name: 'uncaught exception' })) + }) + }) + + it('fails and shows when a promise rejects with a string', () => { + cy.spy(Cypress, 'log').log(false) + const $el = document.createElement('button') + + $el.innerText = `Don't click it!` + $el.addEventListener('click', new Promise((_, reject) => { + reject('Promise rejected with a string!') + })) + + document.querySelector('[data-cy-root]').appendChild($el) + cy.get('button').click().then(() => { + expect(uncaughtExceptionStub).to.have.been.calledOnceWithExactly(null) + expect(Cypress.log).to.be.calledWithMatch(sinon.match({ 'message': `Error: "Promise rejected with a string!"`, name: 'uncaught exception' })) }) }) }) diff --git a/packages/driver/cypress/e2e/cypress/error_utils.cy.ts b/packages/driver/cypress/e2e/cypress/error_utils.cy.ts index f2cad64afd05..f322159903bf 100644 --- a/packages/driver/cypress/e2e/cypress/error_utils.cy.ts +++ b/packages/driver/cypress/e2e/cypress/error_utils.cy.ts @@ -5,6 +5,7 @@ allowTsModuleStubbing() import $stackUtils from '@packages/driver/src/cypress/stack_utils' import $errUtils, { CypressError } from '@packages/driver/src/cypress/error_utils' import $errorMessages from '@packages/driver/src/cypress/error_messages' +import sinon from 'sinon' describe('driver/src/cypress/error_utils', () => { context('.modifyErrMsg', () => { @@ -90,7 +91,7 @@ describe('driver/src/cypress/error_utils', () => { }) it('attaches onFail to the error when it is a function', () => { - const onFail = function () {} + const onFail = function () { } const fn = () => $errUtils.throwErr(new Error('foo'), { onFail }) expect(fn).throw().and.satisfy((err) => { @@ -561,7 +562,7 @@ describe('driver/src/cypress/error_utils', () => { it('does not error if no last log', () => { state.returns({ - getLastLog: () => {}, + getLastLog: () => { }, }) const result = $errUtils.createUncaughtException({ @@ -660,4 +661,24 @@ describe('driver/src/cypress/error_utils', () => { expect(unsupportedPlugin).to.eq(null) }) }) + + context('.logError', () => { + let cypressMock + + beforeEach(() => { + cypressMock = { + log: cy.stub(), + } + }) + + it('calls Cypress.log with error type and message when error is instance of Error', () => { + $errUtils.logError(cypressMock, 'error', new Error('Some error')) + expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: Some error`)) + }) + + it('calls Cypress.log with error type and message when error a string', () => { + $errUtils.logError(cypressMock, 'error', 'Some error') + expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: \"Some error\"`)) + }) + }) }) diff --git a/packages/driver/src/cypress/error_utils.ts b/packages/driver/src/cypress/error_utils.ts index 93eed668203b..4dc370e5048f 100644 --- a/packages/driver/src/cypress/error_utils.ts +++ b/packages/driver/src/cypress/error_utils.ts @@ -549,9 +549,11 @@ const errorFromUncaughtEvent = (handlerType: HandlerType, event) => { errorFromProjectRejectionEvent(event) } -const logError = (Cypress, handlerType: HandlerType, err, handled = false) => { +const logError = (Cypress, handlerType: HandlerType, err: unknown, handled = false) => { + const error = toLoggableError(err) + Cypress.log({ - message: `${err.name}: ${err.message}`, + message: `${error.name}: ${error.message}`, name: 'uncaught exception', type: 'parent', // specifying the error causes the log to be red/failed @@ -572,6 +574,25 @@ const logError = (Cypress, handlerType: HandlerType, err, handled = false) => { }) } +const isLogabbleError = (error: unknown): error is Error => { + return ( + typeof error === 'object' && + error !== null && + 'message' in error && + typeof (error as Record).message === 'string' + ) +} + +const toLoggableError = (maybeError: unknown): Error => { + if (isLogabbleError(maybeError)) return maybeError + + try { + return new Error(JSON.stringify(maybeError)) + } catch { + return new Error(String(maybeError)) + } +} + const getUnsupportedPlugin = (runnable) => { if (!(runnable.invocationDetails && runnable.invocationDetails.originalFile && runnable.err && runnable.err.message)) { return null From 99603099c999483d2da40f04c4648e98f21b8043 Mon Sep 17 00:00:00 2001 From: Gergo Tolnai Date: Fri, 9 Dec 2022 10:18:15 +0100 Subject: [PATCH 2/5] review fixes --- packages/driver/cypress/component/spec.cy.js | 2 +- .../cypress/e2e/cypress/error_utils.cy.ts | 18 ++++++++++++++---- packages/driver/src/cypress/error_utils.ts | 10 ++++++---- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/driver/cypress/component/spec.cy.js b/packages/driver/cypress/component/spec.cy.js index 3300813ac30b..dd75b0c62313 100644 --- a/packages/driver/cypress/component/spec.cy.js +++ b/packages/driver/cypress/component/spec.cy.js @@ -1,4 +1,4 @@ -import sinon from 'sinon' +const { sinon } = Cypress describe('component testing', () => { /** @type {Cypress.Agent} */ diff --git a/packages/driver/cypress/e2e/cypress/error_utils.cy.ts b/packages/driver/cypress/e2e/cypress/error_utils.cy.ts index f322159903bf..036e9623f7a0 100644 --- a/packages/driver/cypress/e2e/cypress/error_utils.cy.ts +++ b/packages/driver/cypress/e2e/cypress/error_utils.cy.ts @@ -671,14 +671,24 @@ describe('driver/src/cypress/error_utils', () => { } }) - it('calls Cypress.log with error type and message when error is instance of Error', () => { + it('calls Cypress.log with error name and message when error is instance of Error', () => { $errUtils.logError(cypressMock, 'error', new Error('Some error')) expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: Some error`)) }) - it('calls Cypress.log with error type and message when error a string', () => { - $errUtils.logError(cypressMock, 'error', 'Some error') - expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: \"Some error\"`)) + it('calls Cypress.log with error name and message when error a string', () => { + $errUtils.logError(cypressMock, 'error', 'Some string error') + expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: \"Some string error\"`)) + }) + + it('calls Cypress.log with default error name and provided message message when error is an object with a message', () => { + $errUtils.logError(cypressMock, 'error', { message: 'Some object error with message' }) + expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: Some object error with message`)) + }) + + it('calls Cypress.log with error name and message when error is an object', () => { + $errUtils.logError(cypressMock, 'error', { err: 'Error details' }) + expect(cypressMock.log).to.have.been.calledWithMatch(sinon.match.has('message', `Error: {"err":"Error details"}`)) }) }) }) diff --git a/packages/driver/src/cypress/error_utils.ts b/packages/driver/src/cypress/error_utils.ts index 4dc370e5048f..da2152fd75a7 100644 --- a/packages/driver/src/cypress/error_utils.ts +++ b/packages/driver/src/cypress/error_utils.ts @@ -553,7 +553,7 @@ const logError = (Cypress, handlerType: HandlerType, err: unknown, handled = fal const error = toLoggableError(err) Cypress.log({ - message: `${error.name}: ${error.message}`, + message: `${error.name || 'Error'}: ${error.message}`, name: 'uncaught exception', type: 'parent', // specifying the error causes the log to be red/failed @@ -574,7 +574,9 @@ const logError = (Cypress, handlerType: HandlerType, err: unknown, handled = fal }) } -const isLogabbleError = (error: unknown): error is Error => { +interface LoggableError { name?: string, message: string } + +const isLoggabbleError = (error: unknown): error is LoggableError => { return ( typeof error === 'object' && error !== null && @@ -583,8 +585,8 @@ const isLogabbleError = (error: unknown): error is Error => { ) } -const toLoggableError = (maybeError: unknown): Error => { - if (isLogabbleError(maybeError)) return maybeError +const toLoggableError = (maybeError: unknown): LoggableError => { + if (isLoggabbleError(maybeError)) return maybeError try { return new Error(JSON.stringify(maybeError)) From 5f2e52ccee6e31314c8051c675e3241a1a3b8b7f Mon Sep 17 00:00:00 2001 From: Gergo Tolnai Date: Fri, 9 Dec 2022 15:06:53 +0100 Subject: [PATCH 3/5] fix sinon import --- packages/driver/cypress/e2e/cypress/error_utils.cy.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/driver/cypress/e2e/cypress/error_utils.cy.ts b/packages/driver/cypress/e2e/cypress/error_utils.cy.ts index 036e9623f7a0..a62012594748 100644 --- a/packages/driver/cypress/e2e/cypress/error_utils.cy.ts +++ b/packages/driver/cypress/e2e/cypress/error_utils.cy.ts @@ -5,7 +5,8 @@ allowTsModuleStubbing() import $stackUtils from '@packages/driver/src/cypress/stack_utils' import $errUtils, { CypressError } from '@packages/driver/src/cypress/error_utils' import $errorMessages from '@packages/driver/src/cypress/error_messages' -import sinon from 'sinon' + +const { sinon } = Cypress describe('driver/src/cypress/error_utils', () => { context('.modifyErrMsg', () => { From cb1fe5dfd9a71e048cf2b3754cb5512c4c5d7b56 Mon Sep 17 00:00:00 2001 From: Gergo Tolnai Date: Thu, 15 Dec 2022 10:26:24 +0100 Subject: [PATCH 4/5] add e2e test --- .../cypress/e2e/runner/reporter.errors.cy.ts | 10 ++++++++ packages/driver/src/cypress/cy.ts | 25 ++++++++++--------- packages/driver/src/cypress/error_utils.ts | 12 +++++---- .../cypress/e2e/errors/uncaught.cy.js | 6 +++++ 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/packages/app/cypress/e2e/runner/reporter.errors.cy.ts b/packages/app/cypress/e2e/runner/reporter.errors.cy.ts index 1d3125047929..eb93884f3f10 100644 --- a/packages/app/cypress/e2e/runner/reporter.errors.cy.ts +++ b/packages/app/cypress/e2e/runner/reporter.errors.cy.ts @@ -180,6 +180,16 @@ describe('errors ui', { ], }) + verify('spec unhandled rejection with string content', { + uncaught: true, + column: 20, + originalMessage: 'Unhandled promise rejection with string content from the spec', + message: [ + 'The following error originated from your test code', + 'It was caused by an unhandled promise rejection', + ], + }) + verify('spec unhandled rejection with done', { uncaught: true, column: 20, diff --git a/packages/driver/src/cypress/cy.ts b/packages/driver/src/cypress/cy.ts index 75ad058a8fd4..4c9b08b40994 100644 --- a/packages/driver/src/cypress/cy.ts +++ b/packages/driver/src/cypress/cy.ts @@ -109,8 +109,8 @@ const setTopOnError = function (Cypress, cy: $Cy) { // prevent Mocha from setting top.onerror Object.defineProperty(top, 'onerror', { - set () {}, - get () {}, + set () { }, + get () { }, configurable: false, enumerable: true, }) @@ -131,12 +131,12 @@ const ensureRunnable = (cy, cmd) => { interface ICyFocused extends Omit< IFocused, 'documentHasFocus' | 'interceptFocus' | 'interceptBlur' -> {} +> { } interface ICySnapshots extends Omit< ISnapshots, 'onCssModified' | 'onBeforeWindowLoad' -> {} +> { } export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssertions, IRetries, IJQuery, ILocation, ITimer, IChai, IXhr, IAliases, ICySnapshots, ICyFocused { id: string @@ -505,16 +505,16 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert // If the runner can communicate, we should setup all events, otherwise just setup the window and fire the load event. if (isRunnerAbleToCommunicateWithAUT) { if (this.Cypress.isBrowser('webkit')) { - // WebKit's unhandledrejection event will sometimes not fire within the AUT - // due to a documented bug: https://bugs.webkit.org/show_bug.cgi?id=187822 - // To ensure that the event will always fire (and always report these - // unhandled rejections to the user), we patch the AUT's Error constructor - // to enqueue a no-op microtask when executed, which ensures that the unhandledrejection - // event handler will be executed if this Error is uncaught. + // WebKit's unhandledrejection event will sometimes not fire within the AUT + // due to a documented bug: https://bugs.webkit.org/show_bug.cgi?id=187822 + // To ensure that the event will always fire (and always report these + // unhandled rejections to the user), we patch the AUT's Error constructor + // to enqueue a no-op microtask when executed, which ensures that the unhandledrejection + // event handler will be executed if this Error is uncaught. const originalError = autWindow.Error autWindow.Error = function __CyWebKitError (...args) { - autWindow.queueMicrotask(() => {}) + autWindow.queueMicrotask(() => { }) return originalError.apply(this, args) } @@ -1059,6 +1059,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert // eslint-disable-next-line @cypress/dev/arrow-body-multiline-braces onError: (handlerType) => (event) => { const { originalErr, err, promise } = $errUtils.errorFromUncaughtEvent(handlerType, event) as ErrorFromProjectRejectionEvent + const handled = cy.onUncaughtException({ err, promise, @@ -1080,7 +1081,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert onSubmit (e) { return cy.Cypress.action('app:form:submitted', e) }, - onLoad () {}, + onLoad () { }, onBeforeUnload (e) { cy.isStable(false, 'beforeunload') diff --git a/packages/driver/src/cypress/error_utils.ts b/packages/driver/src/cypress/error_utils.ts index da2152fd75a7..589f1b32b2ef 100644 --- a/packages/driver/src/cypress/error_utils.ts +++ b/packages/driver/src/cypress/error_utils.ts @@ -189,6 +189,10 @@ const appendErrMsg = (err, errMsg) => { } const makeErrFromObj = (obj) => { + if (_.isString(obj)) { + return new Error(obj) + } + const err2 = new Error(obj.message) err2.name = obj.name @@ -580,18 +584,16 @@ const isLoggabbleError = (error: unknown): error is LoggableError => { return ( typeof error === 'object' && error !== null && - 'message' in error && - typeof (error as Record).message === 'string' - ) + 'message' in error) } const toLoggableError = (maybeError: unknown): LoggableError => { if (isLoggabbleError(maybeError)) return maybeError try { - return new Error(JSON.stringify(maybeError)) + return { message: JSON.stringify(maybeError) } } catch { - return new Error(String(maybeError)) + return { message: String(maybeError) } } } diff --git a/system-tests/project-fixtures/runner-specs/cypress/e2e/errors/uncaught.cy.js b/system-tests/project-fixtures/runner-specs/cypress/e2e/errors/uncaught.cy.js index 8305569c28df..3d80edb27ad7 100644 --- a/system-tests/project-fixtures/runner-specs/cypress/e2e/errors/uncaught.cy.js +++ b/system-tests/project-fixtures/runner-specs/cypress/e2e/errors/uncaught.cy.js @@ -61,6 +61,12 @@ describe('uncaught errors', { defaultCommandTimeout: 0 }, () => { cy.wait(10000) }) + it('spec unhandled rejection with string content', () => { + Promise.reject('Unhandled promise rejection with string content from the spec') + + cy.wait(10000) + }) + // eslint-disable-next-line mocha/handle-done-callback it('spec unhandled rejection with done', (done) => { Promise.reject(new Error('Unhandled promise rejection from the spec')) From ceffe7343c8dbd76327a91e83943758bd5ce0d8c Mon Sep 17 00:00:00 2001 From: Gergo Tolnai Date: Thu, 15 Dec 2022 18:33:28 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Chris Breiding --- packages/app/cypress/e2e/runner/reporter.errors.cy.ts | 2 ++ packages/driver/src/cypress/error_utils.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/app/cypress/e2e/runner/reporter.errors.cy.ts b/packages/app/cypress/e2e/runner/reporter.errors.cy.ts index eb93884f3f10..a1d2797a7079 100644 --- a/packages/app/cypress/e2e/runner/reporter.errors.cy.ts +++ b/packages/app/cypress/e2e/runner/reporter.errors.cy.ts @@ -188,6 +188,8 @@ describe('errors ui', { 'The following error originated from your test code', 'It was caused by an unhandled promise rejection', ], + stackRegex: /.*/, + hasCodeFrame: false, }) verify('spec unhandled rejection with done', { diff --git a/packages/driver/src/cypress/error_utils.ts b/packages/driver/src/cypress/error_utils.ts index 589f1b32b2ef..9389a2cec8b3 100644 --- a/packages/driver/src/cypress/error_utils.ts +++ b/packages/driver/src/cypress/error_utils.ts @@ -580,7 +580,7 @@ const logError = (Cypress, handlerType: HandlerType, err: unknown, handled = fal interface LoggableError { name?: string, message: string } -const isLoggabbleError = (error: unknown): error is LoggableError => { +const isLoggableError = (error: unknown): error is LoggableError => { return ( typeof error === 'object' && error !== null && @@ -588,7 +588,7 @@ const isLoggabbleError = (error: unknown): error is LoggableError => { } const toLoggableError = (maybeError: unknown): LoggableError => { - if (isLoggabbleError(maybeError)) return maybeError + if (isLoggableError(maybeError)) return maybeError try { return { message: JSON.stringify(maybeError) }