Skip to content

Commit

Permalink
Redesign and improve reporter error display (#6467)
Browse files Browse the repository at this point in the history
* redesign and improve reporter error display

- add markdown support
- collapse stacktrace
- separate docs url and add link in reporter

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

* $utils -> $errUtils

* derp

* serializeError -> wrapErr

* cloneErr -> makeErrFromObj

* yarn.lock

* fix unit tests

* move err-model

* fix styles

* fix/improve error logging

* fix non-converted bits

* transfer missed changes

* fix issues

* remove obselete spec

* make type test more reliable

* use should, get retries

* update snapshots

* update e2e network error test

* update more snapshots

* update error whitespace

* update snapshot

* try something out

* nevermind

* fix tooltip

* add some logging

* remove whitespace

* remove spying on window

* update snapshot

* fix test

* update snapshot

* fix merge: snapshot stacktraces

* fix noStackTrace and update snapshot

* update snapshot

* fix yarn.lock

* don't show diff if retrying an existence error

* url -> URL

* don't add newline after docs url and update a few snapshots

* keep opening stack trace from collapsing test

* remove unnecessary global cy reference

* fix tests

* put e2e timeout increase back in the right spot for exit: false

* don't show diff when assertion contains an element

also, keep mocha from messing up extracting error name when it includes a colon

* use backticks for hook error title

* fix appending error message when original message is falsy

* don't show diff on existence failures

* update snapshots

* fix finish/done being called twice due to not returning

* prevent error print button click from propagating

* use correct error methods and remove need for workaround

* create better abstraction around creating cypress error from path, refactor

* fix throwErr and tests

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: Brian Mann <brian.mann86@gmail.com>
Co-authored-by: Ben Kucera <14625260+Bkucera@users.noreply.github.com>
  • Loading branch information
4 people committed Mar 13, 2020
1 parent 1d84887 commit 88673c8
Show file tree
Hide file tree
Showing 121 changed files with 3,340 additions and 2,056 deletions.
2 changes: 1 addition & 1 deletion packages/driver/src/cy/actionability.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ const ensureNotAnimating = function (cy, $el, coordsHistory, animationDistanceTh
// if we dont have at least 2 points
// then automatically retry
if (coordsHistory.length < 2) {
throw $errUtils.cypressErr('coordsHistory must be at least 2 sets of coords')
$errUtils.throwErrByPath('dom.animation_coords_history_invalid')
}

// verify that our element is not currently animating
Expand Down
26 changes: 11 additions & 15 deletions packages/driver/src/cy/chai.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,12 @@ chai.use((chai, u) => {

$chaiJquery(chai, chaiUtils, {
onInvalid (method, obj) {
const err = $errUtils.cypressErr(
$errUtils.errMsgByPath(
'chai.invalid_jquery_obj', {
assertion: method,
subject: $utils.stringifyActual(obj),
},
),
)

throw err
$errUtils.throwErrByPath('chai.invalid_jquery_obj', {
args: {
assertion: method,
subject: $utils.stringifyActual(obj),
},
})
},

onError (err, method, obj, negated) {
Expand Down Expand Up @@ -253,7 +249,7 @@ chai.use((chai, u) => {
return _super.apply(this, arguments)
}

const err = $errUtils.cypressErr($errUtils.errMsgByPath('chai.match_invalid_argument', { regExp }))
const err = $errUtils.cypressErrByPath('chai.match_invalid_argument', { args: { regExp } })

err.retry = false
throw err
Expand Down Expand Up @@ -340,11 +336,11 @@ chai.use((chai, u) => {
return `Not enough elements found. Found '${len1}', expected '${len2}'.`
}

e1.displayMessage = getLongLengthMessage(obj.length, length)
e1.message = getLongLengthMessage(obj.length, length)
throw e1
}

const e2 = $errUtils.cypressErr($errUtils.errMsgByPath('chai.length_invalid_argument', { length }))
const e2 = $errUtils.cypressErrByPath('chai.length_invalid_argument', { args: { length } })

e2.retry = false
throw e2
Expand Down Expand Up @@ -397,10 +393,10 @@ chai.use((chai, u) => {
return `Expected ${node} not to exist in the DOM, but it was continuously found.`
}

return `Expected to find element: '${obj.selector}', but never found it.`
return `Expected to find element: \`${obj.selector}\`, but never found it.`
}

e1.displayMessage = getLongExistsMessage(obj)
e1.message = getLongExistsMessage(obj)
throw e1
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/actions/check.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ checkOrUncheck = (type, subject, values = [], options = {}) ->
if not isAcceptableElement($el)
node = $dom.stringify($el)
word = $utils.plural(options.$el, "contains", "is")
phrase = if type is "check" then " and :radio" else ""
phrase = if type is "check" then " and `:radio`" else ""
$errUtils.throwErrByPath("check_uncheck.invalid_element", {
onFail: options._log
args: { node, word, phrase, cmd: type }
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/actions/scroll.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
## ensure the subject is not window itself
## cause how are you gonna scroll the window into view...
if subject is state("window")
$utils.throwErrByPath("scrollIntoView.subject_is_window")
$errUtils.throwErrByPath("scrollIntoView.subject_is_window")

## throw if we're trying to scroll to multiple elements
if subject.length > 1
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/angular.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
cy.verifyUpcomingAssertions(getEl($elements), options, {
onRetry: resolveElements
onFail: (err) ->
err.displayMessage = "Could not find element for binding: '#{binding}'."
err.message = "Could not find element for binding: '#{binding}'."
})

findByNgAttr = (name, attr, el, options) ->
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/asserting.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
options = {}

if reEventually.test(chainers)
err = $errUtils.cypressErr("The 'eventually' assertion chainer has been deprecated. This is now the default behavior so you can safely remove this word and everything should work as before.")
err = $errUtils.cypressErrByPath('should.eventually_deprecated')
err.retry = false
throwAndLogErr(err)

Expand Down Expand Up @@ -123,7 +123,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->

newExp = _.reduce chainers, (memo, value) =>
if value not of memo
err = $errUtils.cypressErr("The chainer: '#{value}' was not found. Could not build assertion.")
err = $errUtils.cypressErrByPath('should.chainer_not_found', { args: { chainer: value } })
err.retry = false
throwAndLogErr(err)

Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/commands.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ $errUtils = require("../../cypress/error_utils")

command = (ctx, name, args...) ->
if not ctx[name]
cmds = _.keys($Chainer.prototype).join(", ")
cmds = "\`#{_.keys($Chainer.prototype).join("`, `")}\`"
$errUtils.throwErrByPath("miscellaneous.invalid_command", {
args: { name, cmds }
})

ctx[name].apply(window, args)
ctx[name].apply(ctx, args)

module.exports = (Commands, Cypress, cy, state, config) ->
Commands.addChainer({
Expand Down
38 changes: 18 additions & 20 deletions packages/driver/src/cy/commands/connectors.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -211,47 +211,45 @@ module.exports = (Commands, Cypress, cy, state, config) ->
args: { cmd: name }
})

## TODO: use the new error utils that are part of
## the error message enhancements PR
propertyNotOnSubjectErr = (prop) ->
$errUtils.cypressErr(
$errUtils.errMsgByPath("invoke_its.nonexistent_prop", {
$errUtils.cypressErrByPath("invoke_its.nonexistent_prop", {
args: {
prop,
cmd: name
})
)
}
})

propertyValueNullOrUndefinedErr = (prop, value) ->
errMessagePath = if isCmdIts then "its" else "invoke"

$errUtils.cypressErr(
$errUtils.errMsgByPath("#{errMessagePath}.null_or_undefined_prop_value", {
$errUtils.cypressErrByPath("#{errMessagePath}.null_or_undefined_prop_value", {
args: {
prop,
value,
cmd: name
})
)
}
cmd: name
})

subjectNullOrUndefinedErr = (prop, value) ->
errMessagePath = if isCmdIts then "its" else "invoke"

$errUtils.cypressErr(
$errUtils.errMsgByPath("#{errMessagePath}.subject_null_or_undefined", {
$errUtils.cypressErrByPath("#{errMessagePath}.subject_null_or_undefined", {
args: {
prop,
value,
cmd: name
})
)
value,
}
})

propertyNotOnPreviousNullOrUndefinedValueErr = (prop, value, previousProp) ->
$errUtils.cypressErr(
$errUtils.errMsgByPath("invoke_its.previous_prop_null_or_undefined", {
$errUtils.cypressErrByPath("invoke_its.previous_prop_null_or_undefined", {
args: {
prop,
value,
previousProp,
cmd: name
})
)
}
})

traverseObjectAtPath = (acc, pathsArray, index = 0) ->
## traverse at this depth
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ module.exports = function (Commands, Cypress, cy, state, config) {
$errUtils.throwErrByPath('cookies.backend_error', {
args: {
action,
command,
cmd: command,
browserDisplayName: Cypress.browser.displayName,
errMessage: err.message,
errStack: err.stack,
Expand Down
11 changes: 8 additions & 3 deletions packages/driver/src/cy/commands/files.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _ = require("lodash")
Promise = require("bluebird")

$errUtils = require("../../cypress/error_utils")
$errMessages = require("../../cypress/error_messages")

module.exports = (Commands, Cypress, cy, state, config) ->
Commands.addAll({
Expand Down Expand Up @@ -50,16 +51,20 @@ module.exports = (Commands, Cypress, cy, state, config) ->
onFail: (err) ->
return unless err.type is "existence"

if contents?
{ message, docsUrl } = if contents?
## file exists but it shouldn't
err.displayMessage = $errUtils.errMsgByPath("files.existent", {
$errUtils.errObjByPath($errMessages, "files.existent", {
file, filePath
})
else
## file doesn't exist but it should
err.displayMessage = $errUtils.errMsgByPath("files.nonexistent", {
$errUtils.errObjByPath($errMessages, "files.nonexistent", {
file, filePath
})

err.message = message
err.docsUrl = docsUrl

onRetry: verifyAssertions
})

Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/querying.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ module.exports = (Commands, Cypress, cy) => {
get (selector, options = {}) {
const ctx = this

if ((options === null) || Array.isArray(options) || (typeof options !== 'object')) {
if ((options === null) || _.isArray(options) || !_.isPlainObject(options)) {
return $errUtils.throwErrByPath('get.invalid_options', {
args: { options },
})
Expand Down Expand Up @@ -497,7 +497,7 @@ module.exports = (Commands, Cypress, cy) => {

break
case 'existence':
return err.displayMessage = getErr(err)
return err.message = getErr(err)
default:
break
}
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/screenshot.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
isWin = $dom.isWindow(subject)

screenshotConfig = _.pick(options, "capture", "scale", "disableTimersAndAnimations", "blackout", "waitForCommandSynchronization", "padding", "clip", "onBeforeScreenshot", "onAfterScreenshot")
screenshotConfig = $Screenshot.validate(screenshotConfig, "cy.screenshot", options._log)
screenshotConfig = $Screenshot.validate(screenshotConfig, "screenshot", options._log)
screenshotConfig = _.extend($Screenshot.getConfig(), screenshotConfig)

## set this regardless of options.log b/c its used by the
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/traversals.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@ module.exports = (Commands, Cypress, cy, state, config) ->
onFail: (err) ->
if err.type is "existence"
node = $dom.stringify(subject, "short")
err.displayMessage += " Queried from element: #{node}"
})
err.message += " Queried from element: #{node}"
})
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/window.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->

orientationIsValidAndLandscape = (orientation) =>
if orientation not in validOrientations
all = validOrientations.join("' or '")
all = validOrientations.join("` or `")
$errUtils.throwErrByPath "viewport.invalid_orientation", {
onFail: options._log
args: { all, orientation }
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/xhr.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ startXhrServer = (cy, state, config) ->
log.snapshot("response").end()

onNetworkError: (xhr) ->
err = $errUtils.cypressErr($errUtils.errMsgByPath("xhr.network_error"))
err = $errUtils.cypressErrByPath("xhr.network_error")

if log = logs[xhr.id]
log.snapshot("failed").error(err)
Expand Down
6 changes: 4 additions & 2 deletions packages/driver/src/cy/ensures.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ create = (state, expect) ->

if types.length > 1
## append a nice error message telling the user this
err = $errUtils.appendErrMsg(err, "All #{types.length} subject validations failed on this subject.")
errProps = $errUtils.appendErrMsg(err, "All #{types.length} subject validations failed on this subject.")

$errUtils.mergeErrProps(err, errProps)

throw err

Expand Down Expand Up @@ -223,7 +225,7 @@ create = (state, expect) ->
## TODO: REFACTOR THIS TO CALL THE CHAI-OVERRIDES DIRECTLY
## OR GO THROUGH I18N

cy.ensureExistence($el)
ensureExistence($el)

ensureElDoesNotHaveCSS = ($el, cssProperty, cssValue, onFail) ->
cmd = state("current").get("name")
Expand Down
29 changes: 20 additions & 9 deletions packages/driver/src/cy/errors.coffee
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
_ = require("lodash")
$dom = require("../dom")
$errUtils = require("../cypress/error_utils")
$errorMessages = require('../cypress/error_messages')

crossOriginScriptRe = /^script error/i

Expand Down Expand Up @@ -37,28 +39,37 @@ create = (state, config, log) ->
msg = $errUtils.errMsgByPath("uncaught.cross_origin_script")

createErrFromMsg = ->
new Error $errUtils.errMsgByPath("uncaught.error", { msg, source, lineno })
new Error($errUtils.errMsgByPath("uncaught.error", {
msg, source, lineno
}))

## if we have the 5th argument it means we're in a super
## modern browser making this super simple to work with.
err ?= createErrFromMsg()

err.name = "Uncaught " + err.name

suffixMsg = switch type
uncaughtErrLookup = switch type
when "app" then "uncaught.fromApp"
when "spec" then "uncaught.fromSpec"

err = $errUtils.appendErrMsg(err, $errUtils.errMsgByPath(suffixMsg))
uncaughtErrObj = $errUtils.errObjByPath($errorMessages, uncaughtErrLookup)

err.name = "Uncaught " + err.name

uncaughtErrProps = $errUtils.modifyErrMsg(err, uncaughtErrObj.message, (msg1, msg2) ->
return "#{msg1}\n\n#{msg2}"
)
_.defaults(uncaughtErrProps, uncaughtErrObj)

uncaughtErr = $errUtils.mergeErrProps(err, uncaughtErrProps)

err.onFail = ->
uncaughtErr.onFail = ->
if l = current and current.getLastLog()
l.error(err)
l.error(uncaughtErr)

## normalize error message for firefox
$errUtils.normalizeErrorStack(err)
$errUtils.normalizeErrorStack(uncaughtErr)

return err
return uncaughtErr

commandRunningFailed = (err) ->
## allow for our own custom onFail function
Expand Down
21 changes: 13 additions & 8 deletions packages/driver/src/cy/retries.coffee
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
_ = require("lodash")
Promise = require("bluebird")
debug = require('debug')('cypress:driver:retries')

$utils = require("../cypress/utils")
$errUtils = require("../cypress/error_utils")

Expand Down Expand Up @@ -58,15 +59,19 @@ create = (Cypress, state, timeout, clearTimeout, whenStable, finishAssertions) -
if assertions = options.assertions
finishAssertions(assertions)

getErrMessage = (err) ->
_.get(err, 'displayMessage') or
_.get(err, 'message') or
err
{ error, onFail } = options

prependMsg = $errUtils.errMsgByPath("miscellaneous.retry_timed_out")

retryErrProps = $errUtils.modifyErrMsg(error, prependMsg, (msg1, msg2) ->
return "#{msg2}#{msg1}"
)

retryErr = $errUtils.mergeErrProps(error, retryErrProps)

$errUtils.throwErrByPath "miscellaneous.retry_timed_out", {
onFail: (options.onFail or log)
args: { error: getErrMessage(options.error) }
}
$errUtils.throwErr(retryErr, {
onFail: onFail or log
})

runnableHasChanged = ->
## if we've changed runnables don't retry!
Expand Down
Loading

0 comments on commit 88673c8

Please sign in to comment.