fix: handle download from element missing download attribute

cli/CHANGELOG.md

@@ -5,6 +5,8 @@ _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. Note: download behaviors in experimental Webkit are still an issue. Fixes [#14857](https://github.com/cypress-io/cypress/issues/14857).
+- Fixed an issue to account for canceled and failed downloads to correctly reflect these status in Command log as a download failure where previously it would be pending. Fixed in [#28222](https://github.com/cypress-io/cypress/pull/28222).
 - 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).
 - Fixed an issue where network requests made from tabs/windows other than the main Cypress tab would be delayed. Fixes [#28113](https://github.com/cypress-io/cypress/issues/28113).
 - Stopped processing CDP events at the end of a spec when Test Isolation is off and Test Replay is enabled. Addressed in [#28213](https://github.com/cypress-io/cypress/pull/28213).

packages/app/src/runner/event-manager.ts

@@ -148,6 +148,9 @@ export class EventManager {
         case 'complete:download':
           Cypress.downloads.end(data)
           break
+        case 'canceled:download':
+          Cypress.downloads.end(data, true)
+          break
         default:
           break
       }

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', { browser: '!webkit' }, () => {
+    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')
+  })
+})

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>

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"

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.

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])
 

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

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}`,
+        })
+      }
     })
   })
 

packages/extension/test/integration/background_spec.js

@@ -231,6 +231,23 @@ describe('app/background', () => {
       })
     })
 
+    it('onChanged emits automation:push:request canceled:download', async function () {
+      const downloadDelta = {
+        id: '1',
+        state: {
+          current: 'canceled',
+        },
+      }
+
+      sinon.stub(browser.downloads.onChanged, 'addListener').yields(downloadDelta)
+
+      const ws = await this.connect()
+
+      expect(ws.emit).to.be.calledWith('automation:push:request', 'canceled:download', {
+        id: `${downloadDelta.id}`,
+      })
+    })
+
     it('onChanged does not emit if state does not exist', async function () {
       const downloadDelta = {
         id: '1',

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:

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)

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') {
+          return automation.push('complete:download', {
+            id: downloadItem.getETag(),
+          })
+        }
+
+        automation.push('canceled:download', {
           id: downloadItem.getETag(),
         })
       })

packages/server/test/unit/browsers/chrome_spec.js

@@ -338,6 +338,21 @@ describe('lib/browsers/chrome', () => {
           })
         })
       })
+
+      it('pushes canceled:download when download is incomplete', function () {
+        const downloadData = {
+          guid: '1',
+          state: 'canceled',
+        }
+        const options = { downloadsFolder: 'downloads' }
+
+        return this.onCriEvent('Page.downloadProgress', downloadData, options)
+        .then(() => {
+          expect(this.automation.push).to.be.calledWith('canceled:download', {
+            id: '1',
+          })
+        })
+      })
     })
 
     describe('adding header to AUT iframe request', function () {

packages/server/test/unit/browsers/electron_spec.js

@@ -322,7 +322,7 @@ describe('lib/browsers/electron', () => {
         getFilename: () => 'file.csv',
         getMimeType: () => 'text/csv',
         getURL: () => 'http://localhost:1234/file.csv',
-        once: sinon.stub().yields(),
+        once: sinon.stub().yields({}, 'completed'),
       }
 
       this.win.webContents.session.on.withArgs('will-download').yields({}, downloadItem)
@@ -337,6 +337,27 @@ describe('lib/browsers/electron', () => {
       })
     })
 
+    it('pushes canceled:download when download is incomplete', function () {
+      const downloadItem = {
+        getETag: () => '1',
+        getFilename: () => 'file.csv',
+        getMimeType: () => 'text/csv',
+        getURL: () => 'http://localhost:1234/file.csv',
+        once: sinon.stub().yields({}, 'canceled'),
+      }
+
+      this.win.webContents.session.on.withArgs('will-download').yields({}, downloadItem)
+      this.options.downloadsFolder = 'downloads'
+      sinon.stub(this.automation, 'push')
+
+      return electron._launch(this.win, this.url, this.automation, this.options, undefined, undefined, { attachCDPClient: sinon.stub() })
+      .then(() => {
+        expect(this.automation.push).to.be.calledWith('canceled:download', {
+          id: '1',
+        })
+      })
+    })
+
     it('sets download behavior', function () {
       this.options.downloadsFolder = 'downloads'