fix(driver): ensure errors dont get serialized w/ actual/expected when no showDiff (#8527)

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
2026-01-26 08:59:03 -06:00 · 2020-09-14 13:23:42 -04:00
parent 7ccf59562e
commit 35b064add7
8 changed files with 875 additions and 168 deletions
--- a/packages/driver/src/cypress.js
+++ b/packages/driver/src/cypress.js
@@ -1,6 +1,5 @@
 const _ = require('lodash')
 const $ = require('jquery')
-const chai = require('chai')
 const blobUtil = require('blob-util')
 const minimatch = require('minimatch')
 const moment = require('moment')
@@ -347,21 +346,6 @@ class $Cypress {
        break

      case 'runner:fail': {
-        // mocha runner calculated a failure
-        const err = args[0].err
-
-        if (err.type === 'existence' || $dom.isDom(err.actual) || $dom.isDom(err.expected)) {
-          err.showDiff = false
-        }
-
-        if (err.actual) {
-          err.actual = chai.util.inspect(err.actual)
-        }
-
-        if (err.expected) {
-          err.expected = chai.util.inspect(err.expected)
-        }
-
        if (this.config('isTextTerminal')) {
          return this.emit('mocha', 'fail', ...args)
        }
--- a/packages/driver/src/cypress/error_utils.js
+++ b/packages/driver/src/cypress/error_utils.js
@@ -1,12 +1,15 @@
 // See: ./errorScenarios.md for details about error messages and stack traces

 const _ = require('lodash')
+const chai = require('chai')

-const $errorMessages = require('./error_messages')
+const $dom = require('../dom')
 const $utils = require('./utils')
 const $stackUtils = require('./stack_utils')
+const $errorMessages = require('./error_messages')

 const ERROR_PROPS = 'message type name stack sourceMappedStack parsedStack fileName lineNumber columnNumber host uncaught actual expected showDiff isPending docsUrl codeFrame'.split(' ')
+const ERR_PREPARED_FOR_SERIALIZATION = Symbol('ERR_PREPARED_FOR_SERIALIZATION')

 if (!Error.captureStackTrace) {
  Error.captureStackTrace = (err, fn) => {
@@ -16,9 +19,39 @@ if (!Error.captureStackTrace) {
  }
 }

+const prepareErrorForSerialization = (err) => {
+  if (err[ERR_PREPARED_FOR_SERIALIZATION]) {
+    return err
+  }
+
+  if (err.type === 'existence' || $dom.isDom(err.actual) || $dom.isDom(err.expected)) {
+    err.showDiff = false
+  }
+
+  if (err.showDiff === true) {
+    if (err.actual) {
+      err.actual = chai.util.inspect(err.actual)
+    }
+
+    if (err.expected) {
+      err.expected = chai.util.inspect(err.expected)
+    }
+  } else {
+    delete err.actual
+    delete err.expected
+    delete err.showDiff
+  }
+
+  err[ERR_PREPARED_FOR_SERIALIZATION] = true
+
+  return err
+}
+
 const wrapErr = (err) => {
  if (!err) return

+  prepareErrorForSerialization(err)
+
  return $utils.reduceProps(err, ERROR_PROPS)
 }

--- a/packages/runner/snapshots/retries.mochaEvents.spec.js
+++ b/packages/runner/snapshots/retries.mochaEvents.spec.js
--- a/packages/runner/snapshots/runner.mochaEvents.spec.js
+++ b/packages/runner/snapshots/runner.mochaEvents.spec.js
@@ -70,7 +70,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "title": "\"before all\" hook for \"test 1\"",
      "hookName": "before all",
      "hookId": "h1",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "hook",
@@ -86,9 +92,7 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "name": "AssertionError",
      "stack": "match.string",
      "sourceMappedStack": "match.string",
-      "parsedStack": "match.array",
-      "actual": null,
-      "showDiff": false
+      "parsedStack": "match.array"
    }
  ],
  [
@@ -111,7 +115,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 1,
      "title": "test 1",
      "hookName": "before all",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -143,7 +153,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 1,
      "title": "test 1",
      "hookName": "before all",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -276,7 +292,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "title": "\"before each\" hook for \"test 1\"",
      "hookName": "before each",
      "hookId": "h1",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "hook",
@@ -292,9 +314,7 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "name": "AssertionError",
      "stack": "match.string",
      "sourceMappedStack": "match.string",
-      "parsedStack": "match.array",
-      "actual": null,
-      "showDiff": false
+      "parsedStack": "match.array"
    }
  ],
  [
@@ -305,7 +325,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 1,
      "title": "test 1",
      "hookName": "before each",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -349,7 +375,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 1,
      "title": "test 1",
      "hookName": "before each",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -482,7 +514,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "title": "\"after each\" hook for \"test 1\"",
      "hookName": "after each",
      "hookId": "h1",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "hook",
@@ -498,9 +536,7 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "name": "AssertionError",
      "stack": "match.string",
      "sourceMappedStack": "match.string",
-      "parsedStack": "match.array",
-      "actual": null,
-      "showDiff": false
+      "parsedStack": "match.array"
    }
  ],
  [
@@ -511,7 +547,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 1,
      "title": "test 1",
      "hookName": "after each",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -559,7 +601,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 1,
      "title": "test 1",
      "hookName": "after each",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -806,7 +854,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "title": "\"after all\" hook for \"test 2\"",
      "hookName": "after all",
      "hookId": "h1",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "hook",
@@ -822,9 +876,7 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "name": "AssertionError",
      "stack": "match.string",
      "sourceMappedStack": "match.string",
-      "parsedStack": "match.array",
-      "actual": null,
-      "showDiff": false
+      "parsedStack": "match.array"
    }
  ],
  [
@@ -835,7 +887,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 2,
      "title": "test 2",
      "hookName": "after all",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -883,7 +941,13 @@ exports['src/cypress/runner tests finish with correct state hook failures fail i
      "order": 2,
      "title": "test 2",
      "hookName": "after all",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "failedFromHookId": "h1",
      "body": "[body]",
@@ -1105,7 +1169,13 @@ exports['src/cypress/runner tests finish with correct state mocha grep fail with
      "id": "r5",
      "order": 2,
      "title": "test 2",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "test",
@@ -1156,9 +1226,7 @@ exports['src/cypress/runner tests finish with correct state mocha grep fail with
      "name": "AssertionError",
      "stack": "match.string",
      "sourceMappedStack": "match.string",
-      "parsedStack": "match.array",
-      "actual": null,
-      "showDiff": false
+      "parsedStack": "match.array"
    }
  ],
  [
@@ -1234,7 +1302,13 @@ exports['src/cypress/runner tests finish with correct state mocha grep fail with
      "id": "r5",
      "order": 2,
      "title": "test 2",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "test",
@@ -1301,7 +1375,13 @@ exports['src/cypress/runner tests finish with correct state mocha grep fail with
      "id": "r5",
      "order": 2,
      "title": "test 2",
-      "err": "{Object 9}",
+      "err": {
+        "message": "[error message]",
+        "name": "AssertionError",
+        "stack": "match.string",
+        "sourceMappedStack": "match.string",
+        "parsedStack": "match.array"
+      },
      "state": "failed",
      "body": "[body]",
      "type": "test",
--- a/packages/runner/cypress/integration/reporter.errors.spec.js
+++ b/packages/runner/cypress/integration/reporter.errors.spec.js
@@ -1,6 +1,9 @@
 const helpers = require('../support/helpers')

-const { verify } = helpers.createCypress({ config: { isTextTerminal: true, retries: 0 } })
+const { verify } = helpers.createCypress({
+  config: { isTextTerminal: true, retries: 0 },
+  visitUrl: 'http://localhost:3500/fixtures/isolated-runner-inner.html',
+})

 const verifyInternalFailure = (props) => {
  const { method } = props
--- a/packages/runner/cypress/integration/retries.mochaEvents.spec.js
+++ b/packages/runner/cypress/integration/retries.mochaEvents.spec.js
@@ -1,7 +1,7 @@
 const helpers = require('../support/helpers')

 const { shouldHaveTestResults, getRunState, cleanseRunStateMap } = helpers
-const { runIsolatedCypress, snapshotMochaEvents, getAutCypress } = helpers.createCypress({ config: { retries: 2, isTextTerminal: true } })
+const { runIsolatedCypress, snapshotMochaEvents, getAutCypress } = helpers.createCypress({ config: { retries: 2, isTextTerminal: true, firefoxGcInterval: null } })
 const { sinon } = Cypress
 const match = Cypress.sinon.match

@@ -249,6 +249,20 @@ describe('src/cypress/runner retries mochaEvents', () => {
    })
  })

+  // https://github.com/cypress-io/cypress/issues/8363
+  describe('cleanses errors before emitting', () => {
+    it('does not try to serialize error with err.actual as DOM node', () => {
+      runIsolatedCypress(() => {
+        it('visits', () => {
+          cy.visit('/fixtures/dom.html')
+          cy.get('#button').should('not.be.visible')
+        })
+      }, { config: { defaultCommandTimeout: 200 } })
+      // should not have err.actual, expected properties since the subject is a DOM element
+      .then(snapshotMochaEvents)
+    })
+  })
+
  describe('save/reload state', () => {
    const serializeState = () => {
      return getRunState(getAutCypress())
--- a/packages/runner/cypress/support/helpers.js
+++ b/packages/runner/cypress/support/helpers.js
@@ -16,7 +16,6 @@ const eventCleanseMap = {
  parent: stringifyShort,
  tests: stringifyShort,
  commands: stringifyShort,
-  err: stringifyShort,
  invocationDetails: stringifyShort,
  body: '[body]',
  wallClockStartedAt: match.date,
@@ -102,10 +101,11 @@ function createCypress (defaultOptions = {}) {
   * @param {{state?: any, config?: any}} opts
   */
  const runIsolatedCypress = (mochaTestsOrFile, opts = {}) => {
-    _.defaultsDeep(opts, defaultOptions, {
+    opts = _.defaultsDeep(opts, defaultOptions, {
      state: {},
      config: { video: false },
      onBeforeRun () {},
+      visitUrl: 'http://localhost:3500/fixtures/dom.html',
    })

    return cy.visit('/fixtures/isolated-runner.html#/tests/cypress/fixtures/empty_spec.js')
@@ -256,7 +256,7 @@ function createCypress (defaultOptions = {}) {
        .yieldsAsync({ response: {
          isOkStatusCode: true,
          isHtml: true,
-          url: 'http://localhost:3500/fixtures/isolated-runner-inner.html',
+          url: opts.visitUrl,
        } })

        .withArgs('set:runnables')
--- a/packages/server/test/support/helpers/resultsUtils.ts
+++ b/packages/server/test/support/helpers/resultsUtils.ts
@@ -44,7 +44,7 @@ const normalizeTestTimings = function (obj, timings) {
    return
  }

-  return _.set(obj, 'timings', _.mapValues(t, (val, key) => {
+  _.set(obj, 'timings', _.mapValues(t, (val, key) => {
    switch (key) {
      case 'lifecycle':
        // ensure that lifecycle is under 500ms
@@ -112,52 +112,59 @@ export const expectRunsToHaveCorrectTimings = (runs = []) => {
    }

    _.each(run.tests, (test) => {
-      if (test.displayError) {
-        test.displayError = e2e.normalizeStdout(test.displayError)
-      }
-    })
-
-    // now make sure that each tests wallclock duration
-    // is around the sum of all of its timings
-    attempts.forEach((attempt) => {
-      if (attempt.error) {
-        attempt.error.stack = e2e.normalizeStdout(attempt.error.stack).trim()
-      }
-
-      // cannot sum an object, must use array of values
-      const timings = _.sumBy(_.values(attempt.timings), (val) => {
-        if (_.isArray(val)) {
-        // array for hooks
-          return _.sumBy(val, addFnAndAfterFn)
+      try {
+        if (test.displayError) {
+          test.displayError = e2e.normalizeStdout(test.displayError)
        }

-        if (_.isObject(val)) {
-        // obj for test itself
-          return addFnAndAfterFn(val)
-        }
+        const attempts = test.attempts

-        return val
-      })
+        // now make sure that each tests wallclock duration
+        // is around the sum of all of its timings
+        attempts.forEach((attempt) => {
+          if (attempt.error) {
+            attempt.error.stack = e2e.normalizeStdout(attempt.error.stack).trim()
+          }

-      expectDurationWithin(
-        attempt,
-        'wallClockDuration',
-        timings,
-        timings + 80, // add 80ms to account for padding
-        1234,
-      )
+          // cannot sum an object, must use array of values
+          const timings = _.sumBy(_.values(attempt.timings), (val) => {
+            if (_.isArray(val)) {
+              // array for hooks
+              return _.sumBy(val, addFnAndAfterFn)
+            }

-      // now reset all the test timings
-      normalizeTestTimings(attempt, 'timings')
+            if (_.isObject(val)) {
+              // obj for test itself
+              return addFnAndAfterFn(val)
+            }

-      if (attempt.wallClockStartedAt) {
-        const d = new Date(attempt.wallClockStartedAt)
+            return val
+          })

-        expect(d.toJSON()).to.eq(attempt.wallClockStartedAt)
-        attempt.wallClockStartedAt = STATIC_DATE
+          expectDurationWithin(
+            attempt,
+            'wallClockDuration',
+            timings,
+            timings + 80, // add 80ms to account for padding
+            1234,
+          )

-        expect(attempt.videoTimestamp).to.be.a('number')
-        attempt.videoTimestamp = 9999
+          // now reset all the test timings
+          normalizeTestTimings(attempt, 'timings')
+
+          if (attempt.wallClockStartedAt) {
+            const d = new Date(attempt.wallClockStartedAt)
+
+            expect(d.toJSON()).to.eq(attempt.wallClockStartedAt)
+            attempt.wallClockStartedAt = STATIC_DATE
+
+            expect(attempt.videoTimestamp).to.be.a('number')
+            attempt.videoTimestamp = 9999
+          }
+        })
+      } catch (e) {
+        e.message = `Error during validation for test "${test.title.join(' / ')}"\n${e.message}`
+        throw e
      }
    })