Skip to content

Commit

Permalink
Revert "fix: use stdio for CDP instead of TCP (#14348)"
Browse files Browse the repository at this point in the history 
This reverts commit 148a3e1.
  • Loading branch information
@flotwig
flotwig committed Jan 29, 2021
1 parent c3dc7f4 commit 58b40f5
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 365 deletions.
11 changes: 1 addition & 10 deletions packages/launcher/lib/browsers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ export function launch (
browser: FoundBrowser,
url: string,
args: string[] = [],
opts: { pipeStdio?: boolean } = {},
) {
log('launching browser %o', { browser, url })

Expand All @@ -111,15 +110,7 @@ export function launch (

log('spawning browser with args %o', { args })

const stdio = ['ignore', 'pipe', 'pipe']

if (opts.pipeStdio) {
// also pipe stdio 3 and 4 for access to debugger protocol
stdio.push('pipe', 'pipe')
}

// @ts-ignore
const proc = cp.spawn(browser.path, args, { stdio })
const proc = cp.spawn(browser.path, args, { stdio: ['ignore', 'pipe', 'pipe'] })

proc.stdout.on('data', (buf) => {
log('%s stdout: %s', browser.name, String(buf).trim())
Expand Down
67 changes: 2 additions & 65 deletions packages/server/__snapshots__/5_cdp_spec.ts.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
exports['e2e cdp / with TCP transport / handles disconnections as expected'] = `
exports['e2e cdp / handles disconnections as expected'] = `
====================================================================================================
Expand Down Expand Up @@ -59,67 +59,4 @@ Error: connect ECONNREFUSED 127.0.0.1:7777
✖ 1 of 1 failed (100%) XX:XX - - 1 - -
`

exports['e2e cdp / with stdio transport / falls back to connecting via tcp when stdio cannot be connected'] = `
====================================================================================================
(Run Starting)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (spec.ts) │
│ Searched: cypress/integration/spec.ts │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: spec.ts (1 of 1)
Warning: Cypress failed to connect to Chrome via stdio after 1 second. Falling back to TCP...
Connecting to Chrome via TCP was successful, continuing with tests.
passes
✓ passes
1 passing
(Results)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 1 │
│ Passing: 1 │
│ Failing: 0 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0 │
│ Video: true │
│ Duration: X seconds │
│ Spec Ran: spec.ts │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Video)
- Started processing: Compressing to 32 CRF
- Finished processing: /XXX/XXX/XXX/cypress/videos/spec.ts.mp4 (X second)
====================================================================================================
(Run Finished)
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ spec.ts XX:XX 1 1 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 1 1 - - -
`
`
62 changes: 13 additions & 49 deletions packages/server/lib/browsers/chrome.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import * as CriClient from './cri-client'
import * as protocol from './protocol'
import utils from './utils'
import { Browser } from './types'
import errors from '../errors'

// TODO: this is defined in `cypress-npm-api` but there is currently no way to get there
type CypressConfiguration = any
Expand All @@ -38,9 +37,6 @@ type ChromePreferences = {
localState: object
}

const staticCdpPort = Number(process.env.CYPRESS_REMOTE_DEBUGGING_PORT)
const stdioTimeoutMs = Number(process.env.CYPRESS_CDP_TARGET_TIMEOUT) || 60000

const pathToExtension = extension.getPathToExtension()
const pathToTheme = extension.getPathToTheme()

Expand Down Expand Up @@ -177,7 +173,9 @@ const _writeChromePreferences = (userDir: string, originalPrefs: ChromePreferenc
}

const getRemoteDebuggingPort = async () => {
return staticCdpPort || utils.getPort()
const port = Number(process.env.CYPRESS_REMOTE_DEBUGGING_PORT)

return port || utils.getPort()
}

/**
Expand Down Expand Up @@ -245,47 +243,19 @@ const _disableRestorePagesPrompt = function (userDir) {
.catch(() => { })
}

const useStdioCdp = (browser) => {
return (
// CDP via stdio doesn't seem to work in browsers older than 72
// @see https://github.com/cyrus-and/chrome-remote-interface/issues/381#issuecomment-445277683
browser.majorVersion >= 72
// allow users to force TCP by specifying a port in environment
&& !staticCdpPort
)
}

// After the browser has been opened, we can connect to
// its remote interface via a websocket.
const _connectToChromeRemoteInterface = function (browser, process, port, onError) {
const connectTcp = () => {
// @ts-ignore
la(check.userPort(port), 'expected port number to connect CRI to', port)

debug('connecting to Chrome remote interface at random port %d', port)

return protocol.getWsTargetFor(port)
.then((wsUrl) => {
debug('received wsUrl %s for port %d', wsUrl, port)

return CriClient.create({ target: wsUrl }, onError)
})
}

if (!useStdioCdp(browser)) {
return connectTcp()
}

return CriClient.create({ process }, onError)
.timeout(stdioTimeoutMs)
.catch(Bluebird.TimeoutError, async () => {
errors.warning('CDP_STDIO_TIMEOUT', browser.displayName, stdioTimeoutMs)
const _connectToChromeRemoteInterface = function (port, onError) {
// @ts-ignore
la(check.userPort(port), 'expected port number to connect CRI to', port)

const client = await connectTcp()
debug('connecting to Chrome remote interface at random port %d', port)

errors.warning('CDP_FALLBACK_SUCCEEDED', browser.displayName)
return protocol.getWsTargetFor(port)
.then((wsUrl) => {
debug('received wsUrl %s for port %d', wsUrl, port)

return client
return CriClient.create(wsUrl, onError)
})
}

Expand Down Expand Up @@ -465,10 +435,6 @@ export = {
args.push(`--remote-debugging-port=${port}`)
args.push('--remote-debugging-address=127.0.0.1')

if (useStdioCdp(browser)) {
args.push('--remote-debugging-pipe')
}

return args
},

Expand Down Expand Up @@ -524,16 +490,14 @@ export = {
// first allows us to connect the remote interface,
// start video recording and then
// we will load the actual page
const launchedBrowser = await utils.launch(browser, 'about:blank', args, {
pipeStdio: useStdioCdp(browser),
})
const launchedBrowser = await utils.launch(browser, 'about:blank', args)

la(launchedBrowser, 'did not get launched browser instance')

// SECOND connect to the Chrome remote interface
// and when the connection is ready
// navigate to the actual url
const criClient = await this._connectToChromeRemoteInterface(browser, launchedBrowser, port, options.onError)
const criClient = await this._connectToChromeRemoteInterface(port, options.onError)

la(criClient, 'expected Chrome remote interface reference', criClient)

Expand Down
Loading

0 comments on commit 58b40f5

Please sign in to comment.