fix: use stdio for CDP instead of TCP #14348

flotwig · 2020-12-29T21:54:30Z

User facing changelog

Updated the CDP connection to attempt to use the stdio transport first with Chrome 72 and above, before falling back to using TCP. This should remediate issues causing sporadic 'Cypress failed to make a connection to the Chrome DevTools Protocol after retrying' errors.

Additional details

forked chrome-remote-interface to enable sessionId and stdio, see: https://github.com/cypress-io/chrome-remote-interface
- upstream PRs: feat: support for --remote-debugging-pipe transport cyrus-and/chrome-remote-interface#440 feat: add sendRaw function cyrus-and/chrome-remote-interface#441
using CYPRESS_REMOTE_DEBUGGING_PORT will force Cypress to use CDP in TCP mode
warns and falls back to TCP if stdio cannot find a target after 60 seconds (default, can be configured with CYPRESS_CDP_TARGET_TIMEOUT)
- i expect that some users wrap Cypress in a script that will sever stdio, but we'll see
not removing TCP code yet since plugins rely on it/would potentially change our supported Chrome version

How has the user experience changed?

this warning:
CYPRESS_CDP_TARGET_TIMEOUT (undocumented, like CYPRESS_REMOTE_DEBUGGING_PORT, since it's just for testing purposes)

PR Tasks

Have tests been added/updated?
Has the original issue or this PR been tagged with a release in ZenHub?
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?
[na] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2020-12-29T21:54:32Z

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.

jennifer-shehane · 2021-01-04T02:09:13Z

@flotwig There's this newly opened issue with this error happening in Firefox. Would this solution also fix that? #14272

flotwig · 2021-01-04T17:45:36Z

@jennifer-shehane I don't think so, we'd need both Marionette and the Remote Protocol to be available over stdio, and I don't see config options for doing either. Currently the ports are set up like so:

cypress/packages/server/lib/browsers/firefox.ts

Lines 358 to 359 in 814fe6f

    
           defaultLaunchOptions.preferences['devtools.debugger.remote-port'] = foxdriverPort 
        
           defaultLaunchOptions.preferences['marionette.port'] = marionettePort

cypress · 2021-01-11T20:25:38Z

Test summary

3961 • 0 • 52 • 1

Run details


Project	cypress
Status	Passed
Commit	`7a3ace5`
Started	Jan 19, 2021 7:05 PM
Ended	Jan 19, 2021 7:17 PM
Duration	12:00 💡
OS	Linux Debian - 10.5
Browser	Firefox 77

View run in Cypress Dashboard ➡️

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

jennifer-shehane

@flotwig What's the purpose of showing this warning to the user? Can they do anything with this info? IMO, warning should only be displayed if there's an action to advise the user to take.

Warning: Cypress failed to connect to Chrome via stdio after 1 second. Falling back to TCP...

I think this should only display in DEBUG logs. People tend to open issues when if they see warnings, so if the warning will print and then continue on normally - I don't want people opening an issue about this, thinking something is broken.

If the warning is necessary to be shown for some reason to the user, I would prefer to have some sort of 'success' message afterwards to indicate that the connection continued successfully.

Warning: Cypress failed to connect to Chrome via stdio after 1 second. Falling back to TCP...
Connection to TCP successful, continuing with tests

I did not technically review anything else in this PR.

Dismissing my previous review - expecting Zach to update to add a success msg after the warning.

This reverts commit 148a3e1.

… tests (#23522) * detect playwright-webkit browser * fix: use stdio for CDP instead of TCP (#14348) * wip: begin launchin webkit * run mode works w webkit in 10.0 * reset previous cdp changes * run driver webkit tests * always detect webkit in non-prod * fix version detection * actually run new job * cleanup * fix run * try caching pw binary * npx install pw binary * install-deps * add experimentalSessionAndOrigin wk tests * wk experimentalSessionAndOrigin tests * browser icon * fix some tests * reset browsers.ts change * fix more tests * fix even more tests, skip driver CI for now * comma * fix server-unit-test * fix websockets_spec * refactor wkautomation to initialize self from static async method * fix(proxy/prerequests): fix duplicate key behavior, fallthrough * Apply suggestions from code review Co-authored-by: Blue F <blue@cypress.io> * simpler name for StackMap * fix proxy-logging spec, some xhr specs * fix last xhr test * update testConfigOverrides * skip webcam.cy.js * reenable driver tests * ci? * Suggestions from code review * skip remaining failures which won't be fixed here * fix/skip a couple tests * fix tests * skip crashy specs * skip hidden suites * Scoping down range of skipped type tests * Scoping down click test skips * Updating webkit contenteditable selection handling and associated test * Adding additional mouse event filtering when disabled. Validated by opening playwright-webkit outside of cypress and validating logged events when enabled/disabled. * Updating click 'mouseout coords' tests to account for default style changes * Updating a few more click 'mouse state' tests * Getting all click tests passing with no webkit skips. Fixing _most_ type tests, selection focus is troublesome. * Updating cross-origin type action test * Tweaking coords for CI rendering * Adding workaround for webkit default input selection. * Webkit -> WebKit * Adding logic and test for handling capture-phase focus event selections * Type errors tests now passing * Adding a couple more WebKit keyboard/mouse tweaks * Couple more tweaks for special_chars tests. * Updating contenteditable beforeinput event tests * Making WebKit checks more consistent * Don't expose webkit in public types * Adding comments and doing a little cleanup * PR updates * Simplifying workaround for webkit focus selection * Removing unnecessary test * Revert "Removing unnecessary test" This reverts commit 2c52293. * Revert "Simplifying workaround for webkit focus selection" This reverts commit 47d1155. * Removing comment that is no longer applicable * Simplifying selection logic that is now functional for all supported browsers Co-authored-by: Zach Bloomquist <git@chary.us> Co-authored-by: Zach Bloomquist <github@chary.us> Co-authored-by: Blue F <blue@cypress.io>

fix: use stdio for CDP instead of TCP

74b8048

flotwig added 7 commits January 11, 2021 11:46

findTarget

fbb1278

update tests

1113244

update debugging code for ws + stdio compat

09204c1

update e2e test

8435aff

tests

b060e2b

minimize ze diff

02e3b1d

fix cri issue causing next spec to not run

435fb91

flotwig added 2 commits January 11, 2021 15:27

try bumping stdioTimeoutMs

cce7a90

update snapshot

7f41984

flotwig marked this pull request as ready for review January 11, 2021 22:11

flotwig requested review from bahmutov, jennifer-shehane and kuceb January 11, 2021 22:17

jennifer-shehane previously requested changes Jan 15, 2021

View reviewed changes

jennifer-shehane mentioned this pull request Jan 19, 2021

6.3.0 Release cypress-io/cypress-documentation#3470

Merged

4 tasks

add success message

7a3ace5

kuceb approved these changes Jan 19, 2021

View reviewed changes

flotwig merged commit 148a3e1 into develop Jan 19, 2021

nassimerrahoui mentioned this pull request Jan 20, 2021

[Daily-builds] Sometimes Cypress failed to make a connection to the Chrome DevTools Protocol jhipster/generator-jhipster#13635

Closed

1 task

flotwig added a commit that referenced this pull request Jan 29, 2021

Revert "fix: use stdio for CDP instead of TCP (#14348)"

58b40f5

This reverts commit 148a3e1.

flotwig mentioned this pull request Jan 29, 2021

fix: revert "fix: use stdio for CDP instead of TCP" #14819

Merged

pashidlos pushed a commit to pashidlos/cypress that referenced this pull request Jan 30, 2021

fix: use stdio for CDP instead of TCP (cypress-io#14348)

acc63c4

emilong mentioned this pull request Feb 1, 2021

Cypress 6.3.0 Fails to exit if connecting to Chrome falls back to TCP #14701

Closed

watanas mentioned this pull request Feb 1, 2021

Unable to connect to the remote-debugging-port via Chome Dev Protocol (CDP) #14835

Closed

flotwig added a commit that referenced this pull request Mar 16, 2021

fix: use stdio for CDP instead of TCP (#14348)

d0d1736

flotwig deleted the cdp-pipe branch January 24, 2022 18:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use stdio for CDP instead of TCP #14348

fix: use stdio for CDP instead of TCP #14348

flotwig commented Dec 29, 2020 •

edited

Loading

cypress-bot bot commented Dec 29, 2020

jennifer-shehane commented Jan 4, 2021

flotwig commented Jan 4, 2021

cypress bot commented Jan 11, 2021 •

edited

Loading

jennifer-shehane left a comment

fix: use stdio for CDP instead of TCP #14348

fix: use stdio for CDP instead of TCP #14348

Conversation

flotwig commented Dec 29, 2020 • edited Loading

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Dec 29, 2020

jennifer-shehane commented Jan 4, 2021

flotwig commented Jan 4, 2021

cypress bot commented Jan 11, 2021 • edited Loading

Test summary

Run details

jennifer-shehane left a comment

Choose a reason for hiding this comment

flotwig commented Dec 29, 2020 •

edited

Loading

cypress bot commented Jan 11, 2021 •

edited

Loading