Skip to content

Commit

Permalink
fix issues around aborted xhrs (cypress-io#2969)
Browse files Browse the repository at this point in the history
  • Loading branch information
brian-mann authored Dec 19, 2018
1 parent 934ab99 commit 4dcc738
Show file tree
Hide file tree
Showing 5 changed files with 225 additions and 40 deletions.
11 changes: 0 additions & 11 deletions packages/driver/src/cy/commands/xhr.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ server = null
getServer = ->
server ? unavailableErr()

abort = ->
if server
server.abort()

reset = ->
if server
server.restore()
Expand Down Expand Up @@ -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()
Expand Down
61 changes: 44 additions & 17 deletions packages/driver/src/cypress/server.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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("""
Expand Down Expand Up @@ -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) ->
Expand Down Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions packages/driver/test/cypress/integration/commands/xhr_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
111 changes: 111 additions & 0 deletions packages/driver/test/cypress/integration/issues/761_2968_spec.js
Original file line number Diff line number Diff line change
@@ -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')
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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")

0 comments on commit 4dcc738

Please sign in to comment.