fix issues around aborted xhrs

packages/driver/src/cy/commands/xhr.coffee

@@ -12,10 +12,6 @@ server = null
 getServer = ->
   server ? unavailableErr()
 
-abort = ->
-  if server
-    server.abort()
-
 reset = ->
   if server
     server.restore()
@@ -221,13 +217,6 @@ defaults = {
 module.exports = (Commands, Cypress, cy, state, config) ->
   reset()
 
-  ## if our page is going away due to
-  ## a form submit / anchor click then
-  ## we need to cancel all outstanding
-  ## XHR's so the command log displays
-  ## correctly
-  Cypress.on("window:unload", abort)
-
   Cypress.on "test:before:run", ->
     ## reset the existing server
     reset()

packages/driver/src/cypress/server.coffee

@@ -26,6 +26,13 @@ normalize = (val) ->
 
 nope = -> return null
 
+## when the browser naturally cancels/aborts
+## an XHR because the window is unloading
+isAbortedThroughUnload = (xhr) ->
+  xhr.readyState is 4 and
+    xhr.status is 0 and
+      xhr.responseText is ""
+
 warnOnStubDeprecation = (obj, type) ->
   if _.has(obj, "stub")
     $utils.warning("""
@@ -296,39 +303,56 @@ create = (options = {}) ->
       abort  = XHR.prototype.abort
       srh    = XHR.prototype.setRequestHeader
 
-      restoreFn = ->
-        ## restore the property back on the window
-        _.each {send: send, open: open, abort: abort, setRequestHeader: srh}, (value, key) ->
-          XHR.prototype[key] = value
-
-      XHR.prototype.setRequestHeader = ->
-        proxy = server.getProxyFor(@)
+      abortXhr = (xhr) ->
+        proxy = server.getProxyFor(xhr)
 
-        proxy._setRequestHeader.apply(proxy, arguments)
+        ## if the XHR leaks into the next test
+        ## after we've reset our internal server
+        ## then this may be undefined
+        return if not proxy
 
-        srh.apply(@, arguments)
+        ## return if we're already aborted which
+        ## can happen if the browser already canceled
+        ## this xhr but we called abort later
+        return if xhr.aborted
 
-      XHR.prototype.abort = ->
-        ## if we already have a readyState of 4
-        ## then do not get the abort stack or
-        ## set the aborted property or call onXhrAbort
-        ## to test this just use a regular XHR
-        @aborted = true
+        xhr.aborted = true
 
         abortStack = server.getStack()
 
-        proxy = server.getProxyFor(@)
         proxy.aborted = true
 
         options.onXhrAbort(proxy, abortStack)
 
         if _.isFunction(options.onAnyAbort)
-          route = server.getRouteForXhr(@)
+          route = server.getRouteForXhr(xhr)
 
           ## call the onAnyAbort function
           ## after we've called options.onSend
           options.onAnyAbort(route, proxy)
 
+      restoreFn = ->
+        ## restore the property back on the window
+        _.each {send: send, open: open, abort: abort, setRequestHeader: srh}, (value, key) ->
+          XHR.prototype[key] = value
+
+      XHR.prototype.setRequestHeader = ->
+        ## if the XHR leaks into the next test
+        ## after we've reset our internal server
+        ## then this may be undefined
+        if proxy = server.getProxyFor(@)
+          proxy._setRequestHeader.apply(proxy, arguments)
+
+        srh.apply(@, arguments)
+
+      XHR.prototype.abort = ->
+        ## if we already have a readyState of 4
+        ## then do not get the abort stack or
+        ## set the aborted property or call onXhrAbort
+        ## to test this just use a regular XHR
+        if @readyState isnt 4
+          abortXhr(@)
+
         abort.apply(@, arguments)
 
       XHR.prototype.open = (method, url, async = true, username, password) ->
@@ -407,6 +431,9 @@ create = (options = {}) ->
           ## catch synchronous errors caused
           ## by the onreadystatechange function
           try
+            if isAbortedThroughUnload(xhr)
+              abortXhr(xhr)
+
             if _.isFunction(orst = fns.onreadystatechange)
               orst.apply(xhr, arguments)
           catch err

packages/driver/test/cypress/integration/commands/xhr_spec.coffee

@@ -1843,6 +1843,62 @@ describe "src/cy/commands/xhr", ->
       _.each xhrs, (xhr) ->
         expect(xhr.aborted).not.to.be.false
 
+    it "aborts xhrs that haven't been sent", ->
+      cy
+      .window()
+      .then (win) ->
+        xhr = new win.XMLHttpRequest()
+        xhr.open("GET", "/timeout?ms=0")
+        xhr.abort()
+
+        expect(xhr.aborted).to.be.true
+
+    it "aborts xhrs currently in flight", ->
+      log = null
+
+      cy.on "log:changed", (attrs, l) =>
+        if attrs.name is "xhr"
+          log = l
+
+      cy
+      .window()
+      .then (win) ->
+        xhr = new win.XMLHttpRequest()
+        xhr.open("GET", "/timeout?ms=1000")
+        xhr.send()
+        xhr.abort()
+
+        cy.wrap(null).should ->
+          expect(log.get("state")).to.eq("failed")
+          expect(xhr.aborted).to.be.true
+
+    ## https://github.com/cypress-io/cypress/issues/1652
+    it "does not set aborted on XHR's that have completed by have had .abort() called", ->
+      log = null
+
+      cy.on "log:changed", (attrs, l) =>
+        if attrs.name is "xhr"
+          log = l
+
+      cy
+      .window()
+      .then (win) ->
+        new Promise (resolve) ->
+          xhr = new win.XMLHttpRequest()
+          xhr.open("GET", "/timeout?ms=0")
+          xhr.onload = ->
+            xhr.abort()
+            xhr.foo = "bar"
+            resolve(xhr)
+          xhr.send()
+      .then (xhr) ->
+        ## ensure this is set to prevent accidental
+        ## race conditions down the road if something
+        ## goes wrong
+        expect(xhr.foo).to.eq("bar")
+        expect(xhr.aborted).not.to.be.true
+        expect(log.get("state")).to.eq("passed")
+
   context "Cypress.on(window:unload)", ->
     it "aborts all open XHR's", ->
       xhrs = []

packages/driver/test/cypress/integration/issues/761_2968_spec.js

@@ -0,0 +1,111 @@
+// https://github.com/cypress-io/cypress/issues/761
+describe('issue #761 - aborted XHRs from previous tests', () => {
+  context('aborted when complete', () => {
+    it('test 1 dispatches xhr, but completes in test 2', () => {
+      cy.window().then((win) => {
+        const xhr = new win.XMLHttpRequest()
+
+        xhr.open('GET', '/timeout?ms=1000')
+        xhr.onload = () => {
+          // we are in test 2 at this point
+          // and should not throw
+          xhr.abort()
+        }
+        xhr.send()
+      })
+    })
+
+    it('test 2 aborts the completed XHR', () => {
+      cy.wait(2000)
+    })
+  })
+
+  context('aborted before complete', () => {
+    let xhr = null
+
+    // TODO: we lose a reference here to the xhr in test 2
+    // so it shows up as "pending" forever because we reset
+    // the proxied XHR's as references when the next test starts
+    it('test 1 dispatches xhr, but completes in test 2', () => {
+      cy.window().then((win) => {
+        xhr = new win.XMLHttpRequest()
+
+        xhr.open('GET', '/timeout?ms=1000')
+        xhr.send()
+      })
+    })
+
+    it('test 2 aborts the incomplete XHR which is currently in flight', () => {
+      // we are in test 2 at this point
+      // and should not throw when we
+      // abort the incomplete xhr
+      expect(xhr.aborted).not.to.be.true
+
+      xhr.abort()
+    })
+  })
+})
+
+// this tests that XHR references are blown away
+// and no longer invoked when unloading the window
+// and that its unnecessary to abort them
+// https://github.com/cypress-io/cypress/issues/2968
+describe('issue #2968 - unloaded xhrs do not need to be aborted', () => {
+  it('let the browser naturally abort requests without manual intervention on unload', () => {
+    let xhr
+    let log
+
+    const stub = cy.stub()
+
+    cy.on('log:changed', (attrs, l) => {
+      if (attrs.name === 'xhr') {
+        log = l
+      }
+    })
+
+    cy
+    .visit('http://localhost:3500/fixtures/generic.html')
+    .window()
+    .then((win) => {
+      return new Promise((resolve, reject) => {
+        xhr = new win.XMLHttpRequest()
+
+        win.XMLHttpRequest.prototype.abort = stub
+
+        xhr.open('GET', '/timeout?ms=1000')
+        xhr.abort = stub // this should not get called
+        xhr.onerror = stub // this should not fire
+        xhr.onload = stub // this should not fire
+        xhr.onreadystatechange = () => {
+          if (xhr.readyState === 4) {
+            try {
+              // the browser should naturally
+              // abort / cancel this request when
+              // the unload event is called which
+              // should cause this xhr to have
+              // these properties and be displayed
+              // correctly in the Cypress Command Log
+              expect(xhr.aborted).to.be.true
+              expect(xhr.readyState).to.eq(4)
+              expect(xhr.status).to.eq(0)
+              expect(xhr.responseText).to.eq('')
+            } catch (err) {
+              reject(err)
+            }
+
+            resolve()
+          }
+        }
+
+        xhr.send()
+
+        win.location.href = 'about:blank'
+      })
+    })
+    .wrap(null)
+    .should(() => {
+      expect(stub).not.to.be.called
+      expect(log.get('state')).to.eq('failed')
+    })
+  })
+})

packages/server/test/support/fixtures/projects/e2e/cypress/integration/xhr_spec.coffee

@@ -100,16 +100,18 @@ describe "xhrs", ->
 
     it "aborts", ->
       cy
-        .route({
-          method: "POST",
-          url: /users/,
-          response: {name: "b"},
-          delay: 200
-        }).as("createUser")
-        .get("#create").click()
-        .then ->
-          ## simulate an open request which should become
-          ## aborted due to window:unload event
-          Cypress.action("app:window:unload", {})
+        .window()
+        .then (win) ->
+          cy
+          .route({
+            method: "POST",
+            url: /users/,
+            response: {name: "b"},
+            delay: 2000
+          })
+          .as("createUser")
+          .get("#create").click()
+          .then ->
+            win.location.href = '/index.html'
 
-        .wait("@createUser").its("aborted").should("be.true")
+          .wait("@createUser").its("aborted").should("be.true")