Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Socket IO package tries to connect to nowhere at the start #4776

Closed
bahmutov opened this issue Jul 22, 2019 · 2 comments · Fixed by #4799
Closed

Socket IO package tries to connect to nowhere at the start #4776

bahmutov opened this issue Jul 22, 2019 · 2 comments · Fixed by #4799
Assignees
Labels
pkg/socket This is due to an issue in the packages/socket directory

Comments

@bahmutov
Copy link
Contributor

At least Cypress v3.3.2 and v3.4.0 on Mac

Start Cypress in any project with DEBUG=socket* environment variable.
See messages that should not be there

Screen Shot 2019-07-22 at 9 55 24 AM

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 29, 2019

The code for this is done in cypress-io/cypress#4799, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Jul 29, 2019
brian-mann added a commit that referenced this issue Jul 29, 2019
* only connect to socket if path has been replaced, fixes #4776

* upgrade sinon, do not instantiate sandbox

* add tests to ensure connect is not called when not in browser mode

* refactor once again to push connect to a separate module to make testable

* move initializing extension connection to init.js

- prevents situation where connect(…) is called when module is required
- simplifies handling automatic connection only when in the browser
without needing to modify globals


Co-authored-by: Brian Mann <brian.mann86@gmail.com>
flotwig added a commit that referenced this issue Jul 29, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 29, 2019

Released in 3.4.1.

brian-mann added a commit that referenced this issue Sep 24, 2019
* fix specs

* use debugger protocol for cookie handling in electron

* use latest gulp

* use rimraf instead of gulp-clean

* use electron 3.1.8 and node 10.2.1

* use gulp 4 in packages/static

* fix sendCommandAsync, log Schema.getDomains on CDP connect

* autofill e2e test name [skip ci]

* electron@5.0.7, see what new failures exist

* --no-sandbox for launching Electron

* update cookies logic for electron

* node 12

* update snapshot for new node

* update error message for new node

* stub sendCommandAsync

* only connect to socket if path has been replaced, fixes #4776

* update node-sass to support node 12

* skip wacky socket tests for now

* snapshot

* fix run_plugins_spec snapshot, don't include stack trace

* use --no-sandbox on linux to run as root

* allow sendCommandAsync to resolve

* use euid for root check

* log domains even if undefined

* don't worry about ending 1xx responses immediately anymore

* use --max-http-header-size, change max size from 8kb to 1mb, fix #76

* do not send 502 on failed websocket, just send back ECONNRESET

* update websocket spec port to not collide with other test

* update outdated expect

* Revert "only connect to socket if path has been replaced, fixes #4776"

This reverts commit f179eda.

* update gulp in root

* update https-proxy unit tests

* update network spec to properly close server

* update reporter spec

* update https-proxy-agent to fix node 10.10.0 change

discussion: https://github.com/nodejs/node/issues/24474\#issuecomment-511963799

* only pass --max-http-header-size on node >=12

* use own server-destroy implementation that supports secureConnect events

* oops

* update socket_spec

* electron 6.0.0

* console.table introduced in node 10

* change browserify entry to init.js

* handle edge case when no response body

* console.table added in node 10

* do not exit app when all BrowserWindows are closed

* update e2e snapshots

* value may not be null

* update plugins spec

* correct cookie expiry, use browser.getversion for CDP version check

* fix snapshotting for require stacks

* reorder cookies in spec

* warn when depreated electron callback apis are used

* only report 1 plugin error per process

* cleanup

* node 12.4.0, cypress/browsers:node12.4.0-chrome76 docker image

* update shell.openExternal to promisified

* update dialog.showOpenDialog to promisified

* update webContents.session.setProxy to promisified

* updating native dependencies since we don't need ancient node ABI support anymore

* WIP: switch cookies to simpler, jar-less approach

* WIP: switch cookies to simpler, jar-less approach

* making tests pass

* improve cookie filtering logic

* Remove unneeded Promise.try

* filter what makes it to the extension

* properly re-set superdomain cookies on cross-origin cy.visit

* allow comma-separated list of e2e tests

* sort cookies in order of expiration date, ascending

* updating tests, cleanup

* update tests

* version electron as a devDependency, electron@6.0.1

* cleanup, remove old automation

* cleanup, remove old automation

* bump chokidar to fix win10 + node12 issue

was seeing this on windows:
nuxt/nuxt#6035

fixed with version bump

* enable now-supported quit role, re-enable old tests

* don't need that arg there

* remove last deprecated callback electron invocations

* Delete cypress.json

* responding to PR feedback

* cleanup

* invoke

* use 'quit' role

* Use new appMenu role for Cypress menu on mac

* electron@6.0.2

* electron@6.0.3

* remove domain: cookie.domain and see what happens

* remove setErrorHandler

* Revert "remove domain: cookie.domain and see what happens"

This reverts commit 49e9168.

* add unit tests for cookies

* ci

* fix project-content css

* electron@6.0.4

* fix specs_list test

* electron@6.0.7

* some cleanup

* electron@6.0.9

* Update 8_reporters_spec.coffee.js

* electron@5.0.10 - Chromium 73, Node 12

* cli: fix the STDIN pipe on Windows (#5045)

* cli: pipe stdin

* uggh, here is the actual change

* update cli unit tests

* add unit test

* more permissive check for json to include application/vnd.api+j… (#5166)

* more permissive check for json to include

* add json test for content-type application/vnd.api+json

* cruder solution passes e2e tests locally, so let's go with that

* Remove 'charset' from content-type before checking if JSON

* fix eslint for fixture specs (#5176)

* update eslint to lint files within 'fixtures' in support files

- ignore some edge cases like jquery, jsx and obvious js files we wrote
with broken code

* Fixes from eslint to 'fixtures' files

* Catch env variable with reserved name CYPRESS_ENV 1621 (#1626)

* server: check CYPRESS_ENV variable when merging configs

* catch invalid CYPRESS_ENV value in CLI, close #1621

* linting

* sanitize platform in test snapshot

* linting

* update error message text

* add missing comma

* fix finally merge in JS code

* pass CLI linter

* fix log reference, should be debug

* use correct sinon reference

* update message, show first part in red

* update error message text

* Addresses #2953 (#5174)

* Addresses #2953

* Added proper test for new error message

* Didn't realize it ran this test as well, whoops

* Implementing changes as suggested by @jennifer-shehane

* Fixing tests and error output. Moved the checks to the start of the get command to ensure we always catch improper options

* Removing issue test since the querying spec covers it

* Using coffescript isArray check

* depromisify things that were promisified b/t electron 5 <=> 6

Revert "update shell.openExternal to promisified"

This reverts commit 8b6460d.

Revert "update dialog.showOpenDialog to promisified"

This reverts commit 5f178b0.

Revert "update webContents.session.setProxy to promisified"

This reverts commit 727df3a.

* node12.4.0-chrome76 => node12.0.0-chrome75

* fix tests for electron downgrade

* node12.0.0-chrome75 => node12.0.0-chrome73


Co-authored-by: Zach Bloomquist <github@chary.us>
Co-authored-by: Brian Mann <brian.mann86@gmail.com>
grabartley pushed a commit to grabartley/cypress that referenced this issue Oct 6, 2019
* fix specs

* use debugger protocol for cookie handling in electron

* use latest gulp

* use rimraf instead of gulp-clean

* use electron 3.1.8 and node 10.2.1

* use gulp 4 in packages/static

* fix sendCommandAsync, log Schema.getDomains on CDP connect

* autofill e2e test name [skip ci]

* electron@5.0.7, see what new failures exist

* --no-sandbox for launching Electron

* update cookies logic for electron

* node 12

* update snapshot for new node

* update error message for new node

* stub sendCommandAsync

* only connect to socket if path has been replaced, fixes cypress-io#4776

* update node-sass to support node 12

* skip wacky socket tests for now

* snapshot

* fix run_plugins_spec snapshot, don't include stack trace

* use --no-sandbox on linux to run as root

* allow sendCommandAsync to resolve

* use euid for root check

* log domains even if undefined

* don't worry about ending 1xx responses immediately anymore

* use --max-http-header-size, change max size from 8kb to 1mb, fix cypress-io#76

* do not send 502 on failed websocket, just send back ECONNRESET

* update websocket spec port to not collide with other test

* update outdated expect

* Revert "only connect to socket if path has been replaced, fixes cypress-io#4776"

This reverts commit f179eda.

* update gulp in root

* update https-proxy unit tests

* update network spec to properly close server

* update reporter spec

* update https-proxy-agent to fix node 10.10.0 change

discussion: https://github.com/nodejs/node/issues/24474\#issuecomment-511963799

* only pass --max-http-header-size on node >=12

* use own server-destroy implementation that supports secureConnect events

* oops

* update socket_spec

* electron 6.0.0

* console.table introduced in node 10

* change browserify entry to init.js

* handle edge case when no response body

* console.table added in node 10

* do not exit app when all BrowserWindows are closed

* update e2e snapshots

* value may not be null

* update plugins spec

* correct cookie expiry, use browser.getversion for CDP version check

* fix snapshotting for require stacks

* reorder cookies in spec

* warn when depreated electron callback apis are used

* only report 1 plugin error per process

* cleanup

* node 12.4.0, cypress/browsers:node12.4.0-chrome76 docker image

* update shell.openExternal to promisified

* update dialog.showOpenDialog to promisified

* update webContents.session.setProxy to promisified

* updating native dependencies since we don't need ancient node ABI support anymore

* WIP: switch cookies to simpler, jar-less approach

* WIP: switch cookies to simpler, jar-less approach

* making tests pass

* improve cookie filtering logic

* Remove unneeded Promise.try

* filter what makes it to the extension

* properly re-set superdomain cookies on cross-origin cy.visit

* allow comma-separated list of e2e tests

* sort cookies in order of expiration date, ascending

* updating tests, cleanup

* update tests

* version electron as a devDependency, electron@6.0.1

* cleanup, remove old automation

* cleanup, remove old automation

* bump chokidar to fix win10 + node12 issue

was seeing this on windows:
nuxt/nuxt#6035

fixed with version bump

* enable now-supported quit role, re-enable old tests

* don't need that arg there

* remove last deprecated callback electron invocations

* Delete cypress.json

* responding to PR feedback

* cleanup

* invoke

* use 'quit' role

* Use new appMenu role for Cypress menu on mac

* electron@6.0.2

* electron@6.0.3

* remove domain: cookie.domain and see what happens

* remove setErrorHandler

* Revert "remove domain: cookie.domain and see what happens"

This reverts commit 49e9168.

* add unit tests for cookies

* ci

* fix project-content css

* electron@6.0.4

* fix specs_list test

* electron@6.0.7

* some cleanup

* electron@6.0.9

* Update 8_reporters_spec.coffee.js

* electron@5.0.10 - Chromium 73, Node 12

* cli: fix the STDIN pipe on Windows (cypress-io#5045)

* cli: pipe stdin

* uggh, here is the actual change

* update cli unit tests

* add unit test

* more permissive check for json to include application/vnd.api+j… (cypress-io#5166)

* more permissive check for json to include

* add json test for content-type application/vnd.api+json

* cruder solution passes e2e tests locally, so let's go with that

* Remove 'charset' from content-type before checking if JSON

* fix eslint for fixture specs (cypress-io#5176)

* update eslint to lint files within 'fixtures' in support files

- ignore some edge cases like jquery, jsx and obvious js files we wrote
with broken code

* Fixes from eslint to 'fixtures' files

* Catch env variable with reserved name CYPRESS_ENV 1621 (cypress-io#1626)

* server: check CYPRESS_ENV variable when merging configs

* catch invalid CYPRESS_ENV value in CLI, close cypress-io#1621

* linting

* sanitize platform in test snapshot

* linting

* update error message text

* add missing comma

* fix finally merge in JS code

* pass CLI linter

* fix log reference, should be debug

* use correct sinon reference

* update message, show first part in red

* update error message text

* Addresses cypress-io#2953 (cypress-io#5174)

* Addresses cypress-io#2953

* Added proper test for new error message

* Didn't realize it ran this test as well, whoops

* Implementing changes as suggested by @jennifer-shehane

* Fixing tests and error output. Moved the checks to the start of the get command to ensure we always catch improper options

* Removing issue test since the querying spec covers it

* Using coffescript isArray check

* depromisify things that were promisified b/t electron 5 <=> 6

Revert "update shell.openExternal to promisified"

This reverts commit 8b6460d.

Revert "update dialog.showOpenDialog to promisified"

This reverts commit 5f178b0.

Revert "update webContents.session.setProxy to promisified"

This reverts commit 727df3a.

* node12.4.0-chrome76 => node12.0.0-chrome75

* fix tests for electron downgrade

* node12.0.0-chrome75 => node12.0.0-chrome73


Co-authored-by: Zach Bloomquist <github@chary.us>
Co-authored-by: Brian Mann <brian.mann86@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/socket This is due to an issue in the packages/socket directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants