fix: handle page loads when download is triggered #14857

cypress-io · Nov 2, 2023 · 4244eb0 · 4244eb0
1 parent b7a4415
commit 4244eb0
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 18 deletions.
diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md
@@ -5,6 +5,7 @@ _Released 11/7/2023 (PENDING)_
 
 **Bugfixes:**
 
+- Fixed an issue where clicking a link to download a file could cause a page load timeout when the download attribute was missing. Fixes [#14857](https://github.com/cypress-io/cypress/issues/14857).
 - Fixed an issue determining visibility when an element is hidden by an ancestor with a shared edge. Fixes [#27514](https://github.com/cypress-io/cypress/issues/27514).
 
 ## 13.4.0

diff --git a/packages/app/src/runner/event-manager.ts b/packages/app/src/runner/event-manager.ts
@@ -143,12 +143,14 @@ export class EventManager {
           Cypress.Cookies.log(data.message, data.cookie, data.removed)
           break
         case 'create:download':
-          Cypress.cy.isStable(true, 'download')
           Cypress.downloads.start(data)
           break
         case 'complete:download':
           Cypress.downloads.end(data)
           break
+        case 'canceled:download':
+          Cypress.downloads.end(data, true)
+          break
         default:
           break
       }

diff --git a/packages/driver/cypress/e2e/cypress/downloads.cy.ts b/packages/driver/cypress/e2e/cypress/downloads.cy.ts
@@ -4,20 +4,24 @@ describe('src/cypress/downloads', () => {
   let log
   let snapshot
   let end
+  let error
   let downloads
   let downloadItem = {
     id: '1',
     filePath: '/path/to/save/location.csv',
     url: 'http://localhost:1234/location.csv',
     mime: 'text/csv',
   }
+  let action
 
   beforeEach(() => {
     end = cy.stub()
-    snapshot = cy.stub().returns({ end })
+    error = cy.stub()
+    snapshot = cy.stub().returns({ end, error })
     log = cy.stub().returns({ snapshot })
+    action = cy.stub()
 
-    downloads = $Downloads.create({ log })
+    downloads = $Downloads.create({ action, log })
   })
 
   context('#start', () => {
@@ -51,9 +55,21 @@ describe('src/cypress/downloads', () => {
       downloads.start(downloadItem)
       downloads.end({ id: '1' })
 
+      expect(action).to.be.calledWith('app:download:received')
+      expect(snapshot).to.be.called
       expect(end).to.be.called
     })
 
+    it('fails with snapshot if matching log exists', () => {
+      downloads.start(downloadItem)
+      downloads.end({ id: '1' }, true)
+
+      expect(action).to.be.calledWith('app:download:received')
+      expect(snapshot).to.be.called
+      expect(end).not.to.be.called
+      expect(error).to.be.called
+    })
+
     it('is a noop if matching log does not exist', () => {
       downloads.end({ id: '1' })
 
@@ -62,3 +78,35 @@ describe('src/cypress/downloads', () => {
     })
   })
 })
+
+describe('download behavior', () => {
+  beforeEach(() => {
+    cy.visit('/fixtures/downloads.html')
+  })
+
+  it('downloads from anchor tag with download attribute', () => {
+    cy.exec(`rm -f ${Cypress.config('downloadsFolder')}/downloads_records.csv`)
+    cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`).should('not.exist')
+
+    // trigger download
+    cy.get('[data-cy=download-csv]').click()
+    cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`)
+    .should('contain', '"Joe","Smith"')
+  })
+
+  it('downloads from anchor tag without download attribute', () => {
+    cy.exec(`rm -f ${Cypress.config('downloadsFolder')}/downloads_records.csv`)
+    cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`).should('not.exist')
+
+    // trigger download
+    cy.get('[data-cy=download-without-download-attr]').click()
+    cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`)
+    .should('contain', '"Joe","Smith"')
+  })
+
+  it('invalid download path from anchor tag with download attribute', () => {
+    // attempt to download
+    cy.get('[data-cy=invalid-download]').click()
+    cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_does_not_exist.csv`).should('not.exist')
+  })
+})
diff --git a/packages/driver/cypress/fixtures/downloads.html b/packages/driver/cypress/fixtures/downloads.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+  <h3>Download CSV</h3>
+  <a data-cy="download-csv" href="downloads_records.csv" download>downloads_records.csv</a>
+
+  <h3>Download CSV</h3>
+  <a data-cy="download-without-download-attr" href="downloads_records.csv">download csv from anchor tag without download attr</a>
+
+  <h3>Download CSV</h3>
+  <a data-cy="invalid-download" href="downloads_does_not_exist.csv" download>broken download link</a>
+</body>
+</html>
diff --git a/packages/driver/cypress/fixtures/downloads_records.csv b/packages/driver/cypress/fixtures/downloads_records.csv
@@ -0,0 +1,2 @@
+"First name","Last name","Occupation","Age","City","State"
+"Joe","Smith","student",20,"Boston","MA"
diff --git a/packages/driver/src/cy/commands/navigation.ts b/packages/driver/src/cy/commands/navigation.ts
@@ -302,6 +302,23 @@ const stabilityChanged = async (Cypress, state, config, stable) => {
     debug('waiting for window:load')
 
     const promise = new Promise((resolve) => {
+      const handleDownloadUnloadEvent = () => {
+        cy.state('onPageLoadErr', null)
+        cy.isStable(true, 'download')
+
+        options._log
+        .set({
+          message: 'download fired beforeUnload event',
+          consoleProps () {
+            return {
+              Note: 'This event fired when the download was initiated.',
+            }
+          },
+        }).snapshot().end()
+
+        resolve()
+      }
+
       const onWindowLoad = ({ url }) => {
         const href = state('autLocation').href
         const count = getRedirectionCount(href)
@@ -351,6 +368,7 @@ const stabilityChanged = async (Cypress, state, config, stable) => {
         }
       }
 
+      cy.once('download:received', handleDownloadUnloadEvent)
       cy.once('internal:window:load', onInternalWindowLoad)
 
       // If this request is still pending after the test run, resolve it, no commands were waiting on its result.

diff --git a/packages/driver/src/cypress.ts b/packages/driver/src/cypress.ts
@@ -712,6 +712,9 @@ class $Cypress {
       case 'app:navigation:changed':
         return this.emit('navigation:changed', ...args)
 
+      case 'app:download:received':
+        return this.emit('download:received')
+
       case 'app:form:submitted':
         return this.emit('form:submitted', args[0])
 

diff --git a/packages/driver/src/cypress/downloads.ts b/packages/driver/src/cypress/downloads.ts
@@ -25,11 +25,17 @@ export default {
       return log.snapshot()
     }
 
-    const end = ({ id }) => {
+    const end = ({ id }, isCanceled = false) => {
+      Cypress.action('app:download:received')
+
       const log = logs[id]
 
       if (log) {
-        log.snapshot().end()
+        if (isCanceled) {
+          log.snapshot().error(new Error('Download was canceled.'))
+        } else {
+          log.snapshot().end()
+        }
 
         // don't need this anymore since the download has ended
         // and won't change anymore

diff --git a/packages/extension/app/v2/background.js b/packages/extension/app/v2/background.js
@@ -51,11 +51,19 @@ const connect = function (host, path, extraOpts) {
     })
 
     browser.downloads.onChanged.addListener((downloadDelta) => {
-      if ((downloadDelta.state || {}).current !== 'complete') return
+      const state = (downloadDelta.state || {}).current
 
-      ws.emit('automation:push:request', 'complete:download', {
-        id: `${downloadDelta.id}`,
-      })
+      if (state === 'complete') {
+        ws.emit('automation:push:request', 'complete:download', {
+          id: `${downloadDelta.id}`,
+        })
+      }
+
+      if (state === 'canceled') {
+        ws.emit('automation:push:request', 'canceled:download', {
+          id: `${downloadDelta.id}`,
+        })
+      }
     })
   })
 

diff --git a/packages/server/lib/automation/automation.ts b/packages/server/lib/automation/automation.ts
@@ -127,6 +127,7 @@ export class Automation {
         case 'change:cookie':
           return this.cookies.changeCookie(data)
         case 'create:download':
+        case 'canceled:download':
         case 'complete:download':
           return data
         default:

diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts
@@ -298,11 +298,17 @@ const _handleDownloads = async function (client, downloadsFolder: string, automa
   })
 
   client.on('Page.downloadProgress', (data) => {
-    if (data.state !== 'completed') return
+    if (data.state === 'completed') {
+      automation.push('complete:download', {
+        id: data.guid,
+      })
+    }
 
-    automation.push('complete:download', {
-      id: data.guid,
-    })
+    if (data.state === 'canceled') {
+      automation.push('canceled:download', {
+        id: data.guid,
+      })
+    }
   })
 
   await client.send('Page.setDownloadBehavior', {
@@ -521,13 +527,12 @@ export = {
 
     await pageCriClient.send('Page.enable')
 
-    await utils.handleDownloadLinksViaCDP(pageCriClient, automation)
-
     await options['onInitializeNewBrowserTab']?.()
 
     await Promise.all([
       options.videoApi && this._recordVideo(cdpAutomation, options.videoApi, Number(options.browser.majorVersion)),
       this._handleDownloads(pageCriClient, options.downloadsFolder, automation),
+      utils.handleDownloadLinksViaCDP(pageCriClient, automation),
     ])
 
     await this._navigateUsingCRI(pageCriClient, url)

diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts
@@ -337,7 +337,7 @@ export = {
   },
 
   _handleDownloads (win, dir, automation) {
-    const onWillDownload = (event, downloadItem) => {
+    const onWillDownload = (_event, downloadItem) => {
       const savePath = path.join(dir, downloadItem.getFilename())
 
       automation.push('create:download', {
@@ -347,8 +347,14 @@ export = {
         url: downloadItem.getURL(),
       })
 
-      downloadItem.once('done', () => {
-        automation.push('complete:download', {
+      downloadItem.once('done', (_event, state) => {
+        if (state === 'completed') {
+          automation.push('complete:download', {
+            id: downloadItem.getETag(),
+          })
+        }
+
+        automation.push('canceled:download', {
           id: downloadItem.getETag(),
         })
       })