fix: Fix issues with stack traces and command log in Chrome 99 #20049

github-actions · 2022-02-04T08:00:59Z

User facing changelog

Fixes multiple issues in Chrome 99:

No longer throws an exception when attempting to display a Document or ShadowRoot in the Cypress command log.
Stack traces were not reporting properly.

Additional details

Chrome 99 includes an updated version of v8, which breaks our stack trace trimming. A fix is included with commentary on why the change is needed.

It also updates the behavior around enumerating properties of StyleSheets. There's no good reason we should be doing this at all though.

How has the user experience changed?

In older versions, when a ShadowRoot was rendered to the command log, it would treat it as a Javascript object, and enumerate its properties, looking like this:

After this change, it is treated like an HTML element, resulting in displaying this in the command log:

The same is true for a Document. They now render their inner html to the command log rather than a verbose enumeration of their properties (which includes the innerHTML).

cypress-bot · 2022-02-04T08:01:02Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-02-04T08:17:39Z

Test summary

19177 • 0 • 218 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`d2cabd8`
Started	Feb 10, 2022 8:14 AM
Ended	Feb 10, 2022 8:25 AM
Duration	11:39 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	cypress/integration/cypress/proxy-logging_spec.ts		1
1	Proxy Logging > request logging > xhr log has response body/status code

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

…ta-from-98.0.4758.80

BlueWinds · 2022-02-08T16:20:55Z

packages/driver/src/cy/chai/inspect.ts

@@ -124,6 +136,14 @@ export function create (chai) {
      }
    }

+    if (isShadowRoot(value)) {


When we start recursively inspecting objects, listing out their properties, this can include elements with references to objects like window and document. And in Chrome 99, when we iterate over the properties of document.adoptedStyleSheets, it throws an error.

Ultimately though, there's no reason we should be looking over every property of a Document. In the same way we already abort for HTMLElements above, I added an early exit for ShadowRoot and Document, returning a string of their HTML rather than recursively enumerating their properties. This should be a minor performance win, saving on hundreds of calls through formatValue in cases where we pass in window, document or other global-ish objects that aren't literal HTMLElements.

No tests have been added, because the output of this function is already comprehensively tested - dozens of tests failed in Chrome 99 before the changes to this file.

🥳 yay to improved performance!

flotwig

Ran bisect-builds against these changes to find what broke between this beta and stable:

bisect-chrome-builds.py --good=950365 --bad=961656 --command="zsh -c 'cd `pwd`/packages/driver && yarn cypress:run --spec **/type_spec.js --browser %p'" --archive=linux64 --use-local-cache --not-interactive --verify-range

Bad revision: 950808
You are probably looking for a change made after 950807 (known good), but no later than 950808 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/eef8f91d45d7b9d36f0e3cd1b6cd33d3bedff113..e5b482aff3e9e0031527108c7131861ed2640dcc

This is the only commit that changed between good and bad revisions: https://chromium.googlesource.com/chromium/src/+/e5b482aff3e9e0031527108c7131861ed2640dcc

Convert adoptedStyleSheets to use ObservableArray

This CL converts adoptedStyleSheets from a FrozenArray to the
new ObservableArray. This is per the new spec [1]. It should be
backwards compatible, and will allow mutation of adoptedStyleSheets
in place using regular array methods like .push():

  document.adoptedStyleSheets = [sheet1, sheet2];
  document.adoptedStyleSheets.push(sheet);
  ...etc...

Intent to ship:
https://groups.google.com/a/chromium.org/g/blink-dev/c/8p7QvGn3Ezo

[1] https://drafts.csswg.org/cssom/#extensions-to-the-document-or-shadow-root-interface

Bug: 1201744
Bug: 1236777
Change-Id: [Iacbf66a38c0abd477b5628224c97bec9f8a317e4](https://chromium-review.googlesource.com/#/q/Iacbf66a38c0abd477b5628224c97bec9f8a317e4)
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3270424
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#950808}

Apparently it's supposed to be a backwards-compatible change. I wonder why it broke our usage here. Do you think we should file a bug with Chromium?

LGTM otherwise

packages/driver/src/cy/chai/inspect.ts

Co-authored-by: Zach Bloomquist <git@chary.us>

BlueWinds · 2022-02-08T19:38:10Z

Apparently it's supposed to be a backwards-compatible change. I wonder why it broke our usage here. Do you think we should file a bug with Chromium?

LGTM otherwise

I'm fairly convinced this is a browser bug. After spending some time on it, I've reduced it to the following repro case:

for (var a in document.adoptedStyleSheets) {}

This errors on chrome-beta 99, and succeeds on chrome 98. I'll open a bug with Chrome on the subject.

With that said, we should still work around it for now - the change I applied to get the tests passing is a good one to have in place even if chrome fixes the issue at some point.

flotwig · 2022-02-08T19:43:14Z

I think this should also be added to the release and changelog, to call out that we fixed a bug with versions of Chrome after 99.

BlueWinds · 2022-02-08T19:51:00Z

Opened https://bugs.chromium.org/p/chromium/issues/detail?id=1295410. Will update issue to include user-facing changelog.

emilyrohrbough · 2022-02-10T15:41:36Z

packages/driver/src/cy/chai/inspect.ts

@@ -54,6 +54,18 @@ export function create (chai) {
      typeof object.nodeName === 'string'
  }

+  // We can't just check if object instanceof ShadowRoot, because it might be the document of an iframe,
+  // which in Chrome 99+ is a separate class, and instanceof ShadowRoot returns false.
+  const isShadowRoot = function (object) {


Does this change with chrome has impact on the implementation of the cy.shadow() command?

So no tests in the shadom dom specs required changing, but there is a change in the way it's displayed. I'm updating the PR description with details, since it's user-facing.

Unintentional, but I think the new version is much cleaner, so looks like a feature rather than a bug to me!

emilyrohrbough · 2022-02-10T15:46:41Z

packages/driver/src/cypress/stack_utils.ts

+    // (just the error message with no stack trace), then use the original value
+    // instead of the trimmed one.
+    if (newErr.stack.match('\n')) {
+      userInvocationStack = newErr.stack


I'm not understanding this comment 100%, which one is the trimmed one?
Above on line 142, we are setting userInvocationStack = newErr.stack so regardless of this check, won't the stack be the newError stack?

captureStackTrace modifies the error object in place. So for example, beforehand, newErr.stack might will be 15 lines. Then we call captureStackTrace(newErr, captureUserInvocationStack), and now newErr.stack is only 10 lines long.

This guard is against the case - in Chrome 99 - where it was already only two lines, and gets reduced to one. Eg, it might be

Err: Foobar is bad! at (localhost:3500/cypress/stuff/myspec.js:54:12)

and it gets trimmed down to

Err: Foobar is bad!

I'm not 100% sure what caused the changed behavior of captureStackTrace in chromium 99, but the first stack trace is far more useful than the second.

ahhh the fact captureStackTrace modifies this as well was the part I was missing! Thanks for the extra info.

jennifer-shehane

Yah, we'll have to put these changes in the changelog for sure.

@ts-nocheck

* develop: feat: gray out the path to system node in cypress run header (#20121) feat: redesign server errors (#20072) test: fix awesome-typescript-loader test and remove test-binary job (#20131) fix: Fix issues with stack traces and command log in Chrome 99 (#20049) fix: `cy.type(' ')` fires click event on button-like elements. (#20067) fix: `change`, `input` events are not fired when the same option is selected again. (#19623) build: publish vue3 on latest (#20099) chore: release @cypress/webpack-preprocessor-v5.11.1 chore: release @cypress/webpack-dev-server-v1.8.1 fix: detect newly added specs in dev-server compilation (#17950) chore: Remove pkg/driver //@ts-nocheck part 3 (#19837) chore: set up semantic-pull-request GitHub Action (#20091) chore: release @cypress/react-v5.12.2 fix: remove nullish coalescing in js files to support node 12 (#20094) docs: update @cypress/webpack-preprocessor links (#19902) refactor: use aliases instead of meta (#19566)

cypress-bot · 2022-02-15T18:24:29Z

Released in 9.5.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v9.5.0, please open a new issue.

chore: Update Chrome (beta) to 99.0.4844.17

2921c16

github-actions bot requested a review from a team as a code owner February 4, 2022 08:01

github-actions bot requested review from jennifer-shehane and removed request for a team February 4, 2022 08:01

Merge branch 'develop' into update-chrome-stable-from-98.0.4758.80-be…

74e964e

…ta-from-98.0.4758.80

jennifer-shehane removed their request for review February 4, 2022 15:54

davidmunechika assigned BlueWinds Feb 4, 2022

Attempt to fix failures with chrome-beta

ed0ed36

BlueWinds dismissed a stale review via ed0ed36 February 7, 2022 22:12

BlueWinds force-pushed the update-chrome-stable-from-98.0.4758.80-beta-from-98.0.4758.80 branch from 2a4154c to ed0ed36 Compare February 7, 2022 22:12

BlueWinds added 2 commits February 7, 2022 15:17

Fix stack_utils on new chrome beta

6b489e1

Better fix for chrome 99 stack traces

3424f90

BlueWinds reviewed Feb 8, 2022

View reviewed changes

ryanthemanuel requested review from flotwig and emilyrohrbough February 8, 2022 16:49

flotwig previously approved these changes Feb 8, 2022

View reviewed changes

packages/driver/src/cy/chai/inspect.ts Outdated Show resolved Hide resolved

packages/driver/src/cy/chai/inspect.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

ac6af16

Co-authored-by: Zach Bloomquist <git@chary.us>

BlueWinds dismissed flotwig’s stale review via ac6af16 February 8, 2022 19:10

flotwig previously approved these changes Feb 8, 2022

View reviewed changes

BlueWinds changed the title ~~chore: Update Chrome (beta) to 99.0.4844.17~~ fix: Fix issue which Chrome-beta (chrome 99) Feb 8, 2022

github-actions bot changed the title ~~fix: Fix issue which Chrome-beta (chrome 99)~~ chore: Update Chrome (beta) to 99.0.4844.17 Feb 9, 2022

chore: Update Chrome (beta) to 99.0.4844.27

d2cabd8

github-actions bot changed the title ~~chore: Update Chrome (beta) to 99.0.4844.17~~ chore: Update Chrome (beta) to 99.0.4844.27 Feb 10, 2022

github-actions bot dismissed flotwig’s stale review via d2cabd8 February 10, 2022 08:01

emilyrohrbough reviewed Feb 10, 2022

View reviewed changes

BlueWinds requested review from emilyrohrbough and flotwig February 10, 2022 17:17

BlueWinds changed the title ~~chore: Update Chrome (beta) to 99.0.4844.27~~ fix: Fix issues with stack traces and command log in Chrome 99 Feb 10, 2022

emilyrohrbough approved these changes Feb 10, 2022

View reviewed changes

BlueWinds requested a review from jennifer-shehane February 10, 2022 17:21

jennifer-shehane approved these changes Feb 10, 2022

View reviewed changes

BlueWinds merged commit 1db8992 into develop Feb 10, 2022

BlueWinds deleted the update-chrome-stable-from-98.0.4758.80-beta-from-98.0.4758.80 branch February 10, 2022 17:43

cypress-bot bot locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fix issues with stack traces and command log in Chrome 99 #20049

fix: Fix issues with stack traces and command log in Chrome 99 #20049

github-actions bot commented Feb 4, 2022 •

edited by BlueWinds

cypress-bot bot commented Feb 4, 2022

cypress bot commented Feb 4, 2022 •

edited

BlueWinds Feb 8, 2022 •

edited

emilyrohrbough Feb 10, 2022

flotwig left a comment

BlueWinds commented Feb 8, 2022

flotwig commented Feb 8, 2022

BlueWinds commented Feb 8, 2022

emilyrohrbough Feb 10, 2022

BlueWinds Feb 10, 2022

emilyrohrbough Feb 10, 2022

BlueWinds Feb 10, 2022

emilyrohrbough Feb 10, 2022

jennifer-shehane left a comment

cypress-bot bot commented Feb 15, 2022

fix: Fix issues with stack traces and command log in Chrome 99 #20049

fix: Fix issues with stack traces and command log in Chrome 99 #20049

Conversation

github-actions bot commented Feb 4, 2022 • edited by BlueWinds

User facing changelog

Additional details

How has the user experience changed?

cypress-bot bot commented Feb 4, 2022

cypress bot commented Feb 4, 2022 • edited

Test summary

Run details

Flakiness

BlueWinds Feb 8, 2022 • edited

Choose a reason for hiding this comment

emilyrohrbough Feb 10, 2022

Choose a reason for hiding this comment

flotwig left a comment

Choose a reason for hiding this comment

BlueWinds commented Feb 8, 2022

flotwig commented Feb 8, 2022

BlueWinds commented Feb 8, 2022

emilyrohrbough Feb 10, 2022

Choose a reason for hiding this comment

BlueWinds Feb 10, 2022

Choose a reason for hiding this comment

emilyrohrbough Feb 10, 2022

Choose a reason for hiding this comment

BlueWinds Feb 10, 2022

Choose a reason for hiding this comment

emilyrohrbough Feb 10, 2022

Choose a reason for hiding this comment

jennifer-shehane left a comment

Choose a reason for hiding this comment

cypress-bot bot commented Feb 15, 2022

github-actions bot commented Feb 4, 2022 •

edited by BlueWinds

cypress bot commented Feb 4, 2022 •

edited

BlueWinds Feb 8, 2022 •

edited