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

Connect to Chrome Remote Interface when launching Chrome and record video #4628

Merged
merged 108 commits into from
Oct 11, 2019

Conversation

bahmutov
Copy link
Contributor

@bahmutov bahmutov commented Jul 1, 2019

  • closes Add and control Chrome Debugging Protocol #4608
  • partial solution to Support video capture in browsers other than headless electron  #1767 (Headed Chrome browser only)
  • connect and return websocket url (we will ignore the HTTP url for now, but in the future it could be used to directly return browser information to Node)
  • first connect to about:blank, grab the RDI object, then connect to the real url so we have connection (for future logging and video frames) from the very beginning
  • pass the websocket url as a config value to the browser (see note below)
  • tests
  • if ffmpeg video encoding fails for any reason, for example if the browser has sent no frames and there is zero length file to encode, we silently ignore the error, and the user has no idea why there is no video. We should show a warning on non-exit ffmpeg exit code and show STDERR

decision: do not store the references for this PR, it will be decided later when using them

  • do not use domain objects to communicate with remote interface, instead use client.send("command name", ...) format.
  • look into using frame timestamps when writing video rather than wallclock timestamps. Looked, could not figure it if possible at all, asked a question on fluent-ffmpeg repo.

More information

If you need to execute a command via this interface, see CRI API

const CRI = require('chrome-remote-interface')
const client = await CRI({
   target: wsUrl,
   local: true,
})
.timeout(2000)
await client.send('Page.navigate', {url: 'https://github.com'})
  • to see verbose CDP messages use DEBUG=cy-verbose:server:browsers:cri-client namespace (starts with cy-verbose to avoid flooding DEBUG=cypress* namespace)

@bahmutov
Copy link
Contributor Author

bahmutov commented Jul 1, 2019

The order of launching the browser and visiting the site should be (for example for recording video)

  1. spawn chrome on about:blank (remove the url argument)
  2. connect to the debugger protocol / resolve with wsUrl
  3. have node connect to wsUrl and start receiving frames and write them to the ffmpeg stream
  4. navigate the browser to the original URL which is the http://localhost:<port>/__/<spec> via the debugger protocol commands

@bahmutov
Copy link
Contributor Author

bahmutov commented Jul 16, 2019

Trying locally against cypress-test-tiny using Chrome Canary

DEBUG=cypress:server:protocol npm run dev -- --project ~/git/cypress-test-tiny

or for just a run

DEBUG=cypress:server:protocol,cypress:server:browsers \
npm run dev -- --run-project ~/git/cypress-test-tiny --browser chrome

@bahmutov
Copy link
Contributor Author

problematic tests (driver integration)

  • e2e/focus_blur_spec.js
  • issues/1939_1940_2190_spec.js

example build https://circleci.com/gh/cypress-io/cypress/135766

@bahmutov
Copy link
Contributor Author

Interesting: when launching the Chrome browser with url, the focus stays on the new browser tab and our focus tests pass ('can intercept blur/focus events' in focus_blur_spec.js)

Screen Shot 2019-07-17 at 4 34 06 PM

When launching blank page, then directing to url using CRI, the focus goes away from the new tab

Screen Shot 2019-07-17 at 4 33 00 PM

@bahmutov bahmutov changed the title WIP: add Chrome remote interface Connect to Chrome Remote Interface when launching Chrome and save the client Jul 17, 2019
@bahmutov bahmutov requested a review from brian-mann July 17, 2019 21:21
@brian-mann brian-mann changed the title Connect to Chrome Remote Interface when launching Chrome and save the client WIP: Connect to Chrome Remote Interface when launching Chrome and save the client Jul 18, 2019
@brian-mann brian-mann changed the title WIP: Connect to Chrome Remote Interface when launching Chrome and save the client Connect to Chrome Remote Interface when launching Chrome and save the client Jul 18, 2019
@brian-mann brian-mann changed the title Connect to Chrome Remote Interface when launching Chrome and save the client Connect to Chrome Remote Interface when launching Chrome Jul 22, 2019
Copy link
Member

@brian-mann brian-mann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I would like to see us use a dynamic port for the --remote-debugging-port CLI flag and not a hard coded constant. This will conflict if any other automation tools are already using it. There are NPM packages out there already (or you can use the built in node http server) to figure out an open port. The easiest way is to spawn an HTTP server without passing port (it will pick an open one by default) - capture that and then close the HTTP server.

Question:

Is there a reason why we're using the abstractions around the Page.* objects? I would prefer we use the lower level client.send('Domain.method', ...) because that maps directly to the actual RDP documentation - and it's much clearer how the implementation is actually working.

If perhaps the abstraction provides a certain level of gain or backwards compatibility that would be compelling but I'd like to understand what it's doing under the hood first before adopting that pattern.

@flotwig flotwig mentioned this pull request Oct 11, 2019
4 tasks
@brian-mann brian-mann self-requested a review October 11, 2019 19:30
Copy link
Member

@brian-mann brian-mann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • increase _getDelayMsForRetry to 5000ms
  • add a test in run_spec.coffee verifying that the right error is thrown when it cannot be connected to
  • create a nice error message for the user so they understand what happened and why we threw

@flotwig
Copy link
Contributor

flotwig commented Oct 11, 2019

* increase `_getDelayMsForRetry` to 5000ms

* add a test in `run_spec.coffee` verifying that the right error is thrown when it cannot be connected to

* create a nice error message for the user so they understand what happened and why we threw

Added all of this, but instead of adding a test in run_spec, I opted to add an e2e test instead that strips the remote-debugging-port CLI arg to ensure the connection fails. This was simpler than mocking out everything that happens between run.coffee and chrome.coffee and should be easier to maintain since less is stubbed.

@flotwig flotwig requested a review from brian-mann October 11, 2019 20:39
packages/server/lib/errors.coffee Outdated Show resolved Hide resolved
packages/server/test/e2e/2_cdp_spec.ts Outdated Show resolved Hide resolved
@flotwig flotwig requested a review from brian-mann October 11, 2019 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add and control Chrome Debugging Protocol
5 participants