chore: [Multi-domain] Improve timeout error message. #20671

mjhenkes · 2022-03-17T20:40:05Z

Closes Multi-domain: Add errors to guide users to multi-domain APIs #16771

User facing changelog

n/a

Additional details

This PR improves our error message for timeouts due to multi-domain

Errors now include the origin we expected the page to load on as well as the request itself.
Clean up lost page load timeouts that were getting thrown after a test finished.
Page load timeouts thrown from secondary domain are no-longer wrapped as if they were a user spec error.
Spec bridges now listen to the websocket
Moved errors from basic auth to navigation.

Error TODOS:
Better error when a command expects to run on one origin but it is run against another origin.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-03-17T20:40:09Z

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.

mjhenkes · 2022-03-17T20:41:12Z

packages/driver/src/cypress/error_messages.ts

        If so, increase \`redirectionLimit\` value in configuration.`
    },
+    switch_to_domain_load_timed_out ({ ms, configFile, crossOriginUrl, origins }) {
+      return stripIndent`\


error message suggestions are greatly appreciated.

FWIW so far I really like where this has landed

packages/driver/cypress/integration/e2e/multi-domain/navigation_spec.ts

packages/driver/src/cypress/error_messages.ts

packages/driver/src/multi-domain/websocket.ts

cypress · 2022-03-18T18:02:15Z

Test summary

20490 • 0 • 295 • 4 • 4

Run details


Project	cypress
Status	Passed
Commit	`44bc36f`
Started	Mar 21, 2022 4:31 PM
Ended	Mar 21, 2022 4:46 PM
Duration	14:58 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	e2e/multi-domain/navigation_spec.ts	2
1	delayed navigation > localhost -> foobar, delay in
2	errors > never redirects to the subdomain
	reporter.hooks.spec.js	1
1	hooks > can rerun without timeout error leaking into next run (due to run restart)
	cypress/proxy-logging_spec.ts	1
1	Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second

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

packages/driver/cypress/integration/e2e/multi-domain/navigation_spec.ts

packages/driver/src/cypress/error_messages.ts

packages/driver/types/internal-types.d.ts

Co-authored-by: Bill Glesias <bglesias@gmail.com>

packages/driver/src/cy/commands/navigation.ts

packages/driver/src/cy/multi-domain/index.ts

AtofStryker · 2022-03-21T14:27:13Z

packages/driver/src/cypress/error_messages.ts

        If so, increase \`redirectionLimit\` value in configuration.`
    },
+    switch_to_domain_load_timed_out ({ ms, configFile, crossOriginUrl, origins }) {
+      return stripIndent`\


FWIW so far I really like where this has landed

packages/driver/src/cy/commands/navigation.ts

packages/driver/src/cypress/error_messages.ts

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com> Co-authored-by: Bill Glesias <bglesias@gmail.com>

chrisbreiding · 2022-03-21T14:43:14Z

packages/driver/src/cypress/error_messages.ts

+
+        Timed out after waiting \`${ms}ms\` for your remote page to load on origin(s):
+
+        ${origins.map((origin) => `\`${origin}\``).join('\n        ')}


Suggested change

${origins.map((origin) => `\`${origin}\``).join('\n ')}

- ${origins.map((origin) => `\`${origin}\``).join('\n - ')}

Make them list items?

I think this is fine, but wanted to point out that the items will have the dash, we don't seem to do any list item markdown conversion (i tried * previously)

What's an example of there being multiple origins?

I'm planning ahead for nested switchToDomains. Where you may want to return to any of the parent domains.

packages/driver/src/cypress/error_messages.ts

* Better timeout error handling * quick fix, should re-visit * cancel dangling timeouts * re-broadcast the window load event. * clean up code and comment more * ignore better * PR updates * oops, didn't mean to enable this * Apply suggestions from code review Co-authored-by: Bill Glesias <bglesias@gmail.com> * moar pr updates * Apply suggestions from code review Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com> Co-authored-by: Bill Glesias <bglesias@gmail.com> * Fix test * message changes * message update Co-authored-by: Bill Glesias <bglesias@gmail.com> Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

mjhenkes added 7 commits March 16, 2022 13:55

Better timeout error handling

07b340e

Merge branch 'feature-multidomain' into md-page-timeout-error-message

af8b793

quick fix, should re-visit

5e58ce1

cancel dangling timeouts

b7ba0c7

re-broadcast the window load event.

dc3715a

clean up code and comment more

31b18df

ignore better

332db28

mjhenkes added the topic: cy.origin Problems or enhancements related to cy.origin command label Mar 17, 2022

mjhenkes requested review from AtofStryker, chrisbreiding and mschile March 17, 2022 20:40

mjhenkes requested a review from a team as a code owner March 17, 2022 20:40

mjhenkes self-assigned this Mar 17, 2022

mjhenkes removed the request for review from a team March 17, 2022 20:40

mjhenkes commented Mar 17, 2022

View reviewed changes

mschile requested changes Mar 18, 2022

View reviewed changes

Merge branch 'feature-multidomain' into md-page-timeout-error-message

40f52f5

mjhenkes added 2 commits March 18, 2022 14:40

PR updates

a13b41d

oops, didn't mean to enable this

614c9cc

AtofStryker reviewed Mar 18, 2022

View reviewed changes

mjhenkes and others added 3 commits March 18, 2022 13:29

Apply suggestions from code review

b49bced

Co-authored-by: Bill Glesias <bglesias@gmail.com>

moar pr updates

c9be461

Merge branch 'feature-multidomain' into md-page-timeout-error-message

313fb8c

AtofStryker self-requested a review March 21, 2022 14:07

AtofStryker reviewed Mar 21, 2022

View reviewed changes

chrisbreiding reviewed Mar 21, 2022

View reviewed changes

packages/driver/src/cy/commands/navigation.ts Outdated Show resolved Hide resolved

chrisbreiding reviewed Mar 21, 2022

View reviewed changes

packages/driver/src/cypress/error_messages.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

c73e20c

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com> Co-authored-by: Bill Glesias <bglesias@gmail.com>

chrisbreiding reviewed Mar 21, 2022

View reviewed changes

packages/driver/src/cypress/error_messages.ts Outdated Show resolved Hide resolved

Fix test

b1d974a

chrisbreiding reviewed Mar 21, 2022

View reviewed changes

packages/driver/src/cypress/error_messages.ts Outdated Show resolved Hide resolved

mjhenkes added 2 commits March 21, 2022 09:58

message changes

b681412

message update

44bc36f

chrisbreiding approved these changes Mar 21, 2022

View reviewed changes

AtofStryker self-requested a review March 21, 2022 16:36

AtofStryker approved these changes Mar 21, 2022

View reviewed changes

mjhenkes requested a review from mschile March 21, 2022 16:59

mschile approved these changes Mar 21, 2022

View reviewed changes

mjhenkes merged commit 9827b26 into feature-multidomain Mar 21, 2022

mjhenkes deleted the md-page-timeout-error-message branch March 21, 2022 17:55

cypress-bot bot mentioned this pull request Mar 21, 2022

Multi-domain: Add errors to guide users to multi-domain APIs #16771

Closed


		Timed out after waiting \`${ms}ms\` for your remote page to load on origin(s):

		${origins.map((origin) => `\`${origin}\``).join('\n ')}

	${origins.map((origin) => `\`${origin}\``).join('\n ')}
	- ${origins.map((origin) => `\`${origin}\``).join('\n - ')}

chore: [Multi-domain] Improve timeout error message. #20671

chore: [Multi-domain] Improve timeout error message. #20671

Uh oh!

Conversation

mjhenkes commented Mar 17, 2022

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

Uh oh!

cypress-bot bot commented Mar 17, 2022

Uh oh!

mjhenkes Mar 17, 2022

Choose a reason for hiding this comment

Uh oh!

AtofStryker Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cypress bot commented Mar 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test summary

Run details

Flakiness

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AtofStryker Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chrisbreiding Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

mjhenkes Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

chrisbreiding Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

mjhenkes Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cypress bot commented Mar 18, 2022 •

edited

Loading