fix: screenshot() times out when the main Cypress tab is not focused (#29038)

* activate main cypress tab before taking a screenshot

* new tests to cover page activation behavior

* updates changelog

* whitespace

* fix check-ts

* reduce extension failure timeout to 500ms to account for origin bridge timeout

* only use tab activation workaround in chrome; default to Page.bringToFront in headless mode

* update unit tests

* swap order of tests in 5016 system test

* some debugging to try and hunt down firefox issue

* rm debug prev added - looks like sys test passed that time?

* rm debug emit from v2 extension

cli/CHANGELOG.md

@@ -9,6 +9,7 @@ _Released 2/27/2024 (PENDING)_
 
 **Bugfixes:**
 
+- Changed screenshot capture behavior in Chromium to activate the main Cypress tab before capturing. This prevents screenshot capture from timing out in certain situations. Fixed in [#29038](https://github.com/cypress-io/cypress/pull/29038). Fixes [#5016](https://github.com/cypress-io/cypress/issues/5016)
 - Fixed an issue where `.click()` commands on children of disabled elements would still produce "click" events -- even without `{ force: true }`. Fixes [#28788](https://github.com/cypress-io/cypress/issues/28788).
 - Changed RequestBody type to allow for boolean and null literals to be passed as body values. [#28789](https://github.com/cypress-io/cypress/issues/28789)
 

packages/extension/app/v2/background.js

@@ -337,7 +337,6 @@ const automation = {
     })
     .then(fn)
   },
-
 }
 
 module.exports = automation

packages/server/lib/browsers/cdp_automation.ts

@@ -165,10 +165,10 @@ export class CdpAutomation implements CDPClient {
   on: OnFn
   off: OffFn
   send: SendDebuggerCommand
-  private frameTree: any
-  private gettingFrameTree: any
+  private frameTree: Protocol.Page.FrameTree | undefined
+  private gettingFrameTree: Promise<void> | undefined | null
 
-  private constructor (private sendDebuggerCommandFn: SendDebuggerCommand, private onFn: OnFn, private offFn: OffFn, private sendCloseCommandFn: SendCloseCommand, private automation: Automation) {
+  private constructor (private sendDebuggerCommandFn: SendDebuggerCommand, private onFn: OnFn, private offFn: OffFn, private sendCloseCommandFn: SendCloseCommand, private automation: Automation, private focusTabOnScreenshot: boolean = false, private isHeadless: boolean = false) {
     onFn('Network.requestWillBeSent', this.onNetworkRequestWillBeSent)
     onFn('Network.responseReceived', this.onResponseReceived)
     onFn('Network.requestServedFromCache', this.onRequestServedFromCache)
@@ -197,14 +197,62 @@ export class CdpAutomation implements CDPClient {
     await this.sendDebuggerCommandFn('Page.startScreencast', screencastOpts)
   }
 
-  static async create (sendDebuggerCommandFn: SendDebuggerCommand, onFn: OnFn, offFn: OffFn, sendCloseCommandFn: SendCloseCommand, automation: Automation, protocolManager?: ProtocolManagerShape): Promise<CdpAutomation> {
-    const cdpAutomation = new CdpAutomation(sendDebuggerCommandFn, onFn, offFn, sendCloseCommandFn, automation)
+  static async create (sendDebuggerCommandFn: SendDebuggerCommand, onFn: OnFn, offFn: OffFn, sendCloseCommandFn: SendCloseCommand, automation: Automation, protocolManager?: ProtocolManagerShape, focusTabOnScreenshot: boolean = false, isHeadless?: boolean): Promise<CdpAutomation> {
+    const cdpAutomation = new CdpAutomation(sendDebuggerCommandFn, onFn, offFn, sendCloseCommandFn, automation, focusTabOnScreenshot, isHeadless)
 
     await sendDebuggerCommandFn('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS)
 
     return cdpAutomation
   }
 
+  private async activateMainTab () {
+    const ActivationTimeoutMessage = 'Unable to communicate with Cypress Extension'
+
+    const sendActivationMessage = `
+      (() => {
+        if (document.defaultView !== top) { return Promise.resolve() }
+        return new Promise((res) => {
+          const onMessage = (ev) => {
+            if (ev.data.message === 'cypress:extension:main:tab:activated') {
+              window.removeEventListener('message', onMessage)
+              res()
+            }
+          }
+
+          window.addEventListener('message', onMessage)
+          window.postMessage({ message: 'cypress:extension:activate:main:tab' })
+        })
+      })()`
+
+    if (this.isHeadless) {
+      debugVerbose('Headless, so bringing page to front instead of negotiating with extension')
+      await this.sendDebuggerCommandFn('Page.bringToFront')
+    } else {
+      try {
+        debugVerbose('sending activation message ', sendActivationMessage)
+        await Promise.race([
+          this.sendDebuggerCommandFn('Runtime.evaluate', {
+            expression: sendActivationMessage,
+            awaitPromise: true,
+          }),
+          new Promise((_, reject) => {
+            setTimeout(() => reject(new Error(ActivationTimeoutMessage)), 500)
+          }),
+        ])
+      } catch (e) {
+        debugVerbose('Error occurred while attempting to activate main tab: ', e)
+        // If rejected due to timeout, fall back to bringing the main tab to focus -
+        // this will steal window focus, so it is a last resort. If any other error
+        // was thrown, re-throw as it was unexpected.
+        if ((e as Error).message === ActivationTimeoutMessage) {
+          await this.sendDebuggerCommandFn('Page.bringToFront')
+        } else {
+          throw e
+        }
+      }
+    }
+  }
+
   private onNetworkRequestWillBeSent = async (params: Protocol.Network.RequestWillBeSentEvent) => {
     debugVerbose('received networkRequestWillBeSent %o', params)
 
@@ -420,7 +468,7 @@ export class CdpAutomation implements CDPClient {
     client.on('Page.frameDetached', this._updateFrameTree(client, 'Page.frameDetached'))
   }
 
-  onRequest = (message, data) => {
+  onRequest = async (message, data) => {
     let setCookie
 
     switch (message) {
@@ -494,6 +542,16 @@ export class CdpAutomation implements CDPClient {
       case 'remote:debugger:protocol':
         return this.sendDebuggerCommandFn(data.command, data.params, data.sessionId)
       case 'take:screenshot':
+        debugVerbose('capturing screenshot')
+
+        if (this.focusTabOnScreenshot) {
+          try {
+            await this.activateMainTab()
+          } catch (e) {
+            debugVerbose('Error while attempting to activate main tab: %O', e)
+          }
+        }
+
         return this.sendDebuggerCommandFn('Page.captureScreenshot', { format: 'png' })
         .catch((err) => {
           throw new Error(`The browser responded with an error when Cypress attempted to take a screenshot.\n\nDetails:\n${err.message}`)

packages/server/lib/browsers/chrome.ts

@@ -325,7 +325,7 @@ const _handleDownloads = async function (client, downloadsFolder: string, automa
 let onReconnect: (client: CriClient) => Promise<void> = async () => undefined
 
 const _setAutomation = async (client: CriClient, automation: Automation, resetBrowserTargets: (shouldKeepTabOpen: boolean) => Promise<void>, options: BrowserLaunchOpts) => {
-  const cdpAutomation = await CdpAutomation.create(client.send, client.on, client.off, resetBrowserTargets, automation, options.protocolManager)
+  const cdpAutomation = await CdpAutomation.create(client.send, client.on, client.off, resetBrowserTargets, automation, options.protocolManager, true, options.isTextTerminal)
 
   automation.use(cdpAutomation)
 

packages/server/test/unit/browsers/cdp_automation_spec.ts

@@ -465,20 +465,78 @@ context('lib/browsers/cdp_automation', () => {
     })
 
     describe('take:screenshot', () => {
-      it('resolves with base64 data URL', function () {
+      beforeEach(function () {
         this.sendDebuggerCommand.withArgs('Browser.getVersion').resolves({ protocolVersion: '1.3' })
-        this.sendDebuggerCommand.withArgs('Page.captureScreenshot').resolves({ data: 'foo' })
-
-        return expect(this.onRequest('take:screenshot'))
-        .to.eventually.equal('data:image/png;base64,foo')
       })
 
-      it('rejects nicely if Page.captureScreenshot fails', function () {
-        this.sendDebuggerCommand.withArgs('Browser.getVersion').resolves({ protocolVersion: '1.3' })
-        this.sendDebuggerCommand.withArgs('Page.captureScreenshot').rejects()
+      describe('when tab focus behavior default (disabled)', function () {
+        it('resolves with base64 data URL', function () {
+          this.sendDebuggerCommand.withArgs('Page.captureScreenshot').resolves({ data: 'foo' })
 
-        return expect(this.onRequest('take:screenshot'))
-        .to.be.rejectedWith('The browser responded with an error when Cypress attempted to take a screenshot.')
+          return expect(this.onRequest('take:screenshot'))
+          .to.eventually.equal('data:image/png;base64,foo')
+        })
+
+        it('rejects nicely if Page.captureScreenshot fails', function () {
+          this.sendDebuggerCommand.withArgs('Page.captureScreenshot').rejects()
+
+          return expect(this.onRequest('take:screenshot'))
+          .to.be.rejectedWith('The browser responded with an error when Cypress attempted to take a screenshot.')
+        })
+      })
+
+      describe('when tab focus behavior is enabled', function () {
+        let requireTabFocus
+        let isHeadless
+
+        beforeEach(() => {
+          requireTabFocus = true
+        })
+
+        describe('when headless', () => {
+          beforeEach(() => {
+            isHeadless = true
+          })
+
+          it('does not try to comm with extension, simply brings page to front', async function () {
+            cdpAutomation = await CdpAutomation.create(this.sendDebuggerCommand, this.onFn, this.offFn, this.sendCloseTargetCommand, this.automation, undefined, requireTabFocus, isHeadless)
+            this.sendDebuggerCommand.withArgs('Page.captureScreenshot').resolves({ data: 'foo' })
+
+            expect(cdpAutomation.onRequest('take:screenshot', undefined)).to.eventually.equal('data:image/png;base64,foo')
+            expect(this.sendDebuggerCommand).not.to.be.calledWith('Runtime.evaluate')
+            expect(this.sendDebuggerCommand).to.be.calledWith('Page.bringToFront')
+          })
+        })
+
+        describe('when not headless', () => {
+          beforeEach(async function () {
+            isHeadless = false
+            cdpAutomation = await CdpAutomation.create(this.sendDebuggerCommand, this.onFn, this.offFn, this.sendCloseTargetCommand, this.automation, undefined, requireTabFocus, isHeadless)
+            this.sendDebuggerCommand.withArgs('Page.captureScreenshot').resolves({ data: 'foo' })
+          })
+
+          describe('and the extension activates the tab', function () {
+            beforeEach(function () {
+              this.sendDebuggerCommand.withArgs('Runtime.evaluate').resolves()
+              this.sendDebuggerCommand.withArgs('Page.captureScreenshot').resolves({ data: 'foo' })
+            })
+
+            it('captures the screenshot', function () {
+              expect(cdpAutomation.onRequest('take:screenshot', undefined)).to.eventually.equal('data:image/png;base64,foo')
+            })
+          })
+
+          describe('and the extension fails to activate the tab', function () {
+            beforeEach(function () {
+              this.sendDebuggerCommand.withArgs('Runtime.evaluate').rejects(new Error('Unable to communicate with Cypress Extension'))
+              this.sendDebuggerCommand.withArgs('Page.bringToFront').resolves()
+            })
+
+            it('captures the screenshot', function () {
+              expect(cdpAutomation.onRequest('take:screenshot', undefined)).to.eventually.equal('data:image/png;base64,foo')
+            })
+          })
+        })
       })
     })
 

system-tests/__snapshots__/issue_5016_spec.js

@@ -0,0 +1,74 @@
+exports['e2e issue 5016 - screenshot times out after clicking target _blank / fails but does not timeout taking screenshot'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (issue-5016.cy.js)                                                         │
+  │ Searched:   cypress/e2e/**/*.cy.{js,jsx,ts,tsx}                                                │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+                                                                                                    
+  Running:  issue-5016.cy.js                                                                (1 of 1)
+
+
+  issue 5016
+    ✓ should take a normal screenshot
+    1) should fail but not timeout while taking the screenshot
+    ✓ should not timeout taking screenshot when not failing
+
+
+  2 passing
+  1 failing
+
+  1) issue 5016
+       should fail but not timeout while taking the screenshot:
+     AssertionError: Timed out retrying after 4050ms: expected '<a>' to have attribute 'foo'
+      [stack trace lines]
+
+
+
+
+  (Results)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Tests:        3                                                                                │
+  │ Passing:      2                                                                                │
+  │ Failing:      1                                                                                │
+  │ Pending:      0                                                                                │
+  │ Skipped:      0                                                                                │
+  │ Screenshots:  3                                                                                │
+  │ Video:        false                                                                            │
+  │ Duration:     X seconds                                                                        │
+  │ Spec Ran:     issue-5016.cy.js                                                                 │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+  (Screenshots)
+
+  -  /XXX/XXX/XXX/cypress/screenshots/issue-5016.cy.js/issue 5016 -- should take a no          (YxX)
+     rmal screenshot.png                                                                            
+  -  /XXX/XXX/XXX/cypress/screenshots/issue-5016.cy.js/issue 5016 -- should fail but           (YxX)
+     not timeout while taking the screenshot (failed).png                                           
+  -  /XXX/XXX/XXX/cypress/screenshots/issue-5016.cy.js/issue 5016 -- should not timeo          (YxX)
+     ut taking screenshot when not failing.png                                                      
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+       Spec                                              Tests  Passing  Failing  Pending  Skipped  
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✖  issue-5016.cy.js                         XX:XX        3        2        1        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    ✖  1 of 1 failed (100%)                     XX:XX        3        2        1        -        -  
+
+
+`

system-tests/projects/config-screenshot-on-failure-enabled/cypress.config.js

@@ -0,0 +1,6 @@
+module.exports = {
+  screenshotOnRunFailure: true,
+  e2e: {
+    supportFile: false,
+  },
+}

system-tests/projects/config-screenshot-on-failure-enabled/cypress/e2e/issue-5016.cy.js

@@ -0,0 +1,18 @@
+// https://github.com/cypress-io/cypress/issues/5016
+describe('issue 5016', {
+  screenshotOnRunFailure: true,
+}, function () {
+  it('should take a normal screenshot', function () {
+    cy.visit('/cypress/fixtures/issue-5016/index.html').screenshot()
+  })
+
+  it('should fail but not timeout while taking the screenshot', function () {
+    cy.visit('/cypress/fixtures/issue-5016/index.html')
+    cy.get('a').click().should('have.attr', 'foo')
+  })
+
+  it('should not timeout taking screenshot when not failing', function () {
+    cy.visit('/cypress/fixtures/issue-5016/index.html')
+    cy.get('a').click().screenshot()
+  })
+})

system-tests/projects/config-screenshot-on-failure-enabled/cypress/fixtures/issue-5016/index.html

@@ -0,0 +1,5 @@
+<!doctype html>
+
+<body>
+  <a target="_blank" href="./new.html">New Tab</a>
+</body>

system-tests/projects/config-screenshot-on-failure-enabled/cypress/fixtures/issue-5016/new.html

@@ -0,0 +1,3 @@
+<!doctype html>
+
+<html></html>

system-tests/test/issue_5016_spec.js

@@ -0,0 +1,13 @@
+const systemTests = require('../lib/system-tests').default
+
+describe('e2e issue 5016 - screenshot times out after clicking target _blank', function () {
+  systemTests.setup()
+
+  systemTests.it('fails but does not timeout taking screenshot', {
+    project: 'config-screenshot-on-failure-enabled',
+    sanitizeScreenshotDimensions: true,
+    snapshot: true,
+    expectedExitCode: 1,
+    browser: '!webkit',
+  })
+})