fix: Canceled Intercepted calls will now end a waited on alias (#24709)

* fix: on a canceled request, end waiting on an intercepted alias

* Add tests, fix ts

* skip firefox

* add doc

* try to fix flake

* delay?

* Use http proxy instead of cdp.

* 'fix' safari

* test updates

* PR updates

* test updates
This commit is contained in:
Matt Henkes
2022-11-25 08:53:40 -06:00
committed by GitHub
parent 497c0548dc
commit b04f9a1143
10 changed files with 270 additions and 45 deletions
@@ -20,6 +20,60 @@ const uniqueRoute = (route) => {
return `${route}-${routeCount}`
}
describe('network stubbing - not skipped', () => {
it('stops waiting when an xhr request is canceled', () => {
cy.visit('http://localhost:3500/fixtures/generic.html')
cy.intercept('POST', /users/, {
body: { name: 'b' },
delay: 2000,
}).as('createUser')
cy.window()
.then((win) => {
const xhr = new win.XMLHttpRequest()
xhr.open('POST', '/users/')
xhr.send()
win.location.reload()
cy.wait('@createUser').its('state').should('eq', 'Errored')
})
})
it('stops waiting when an fetch request is canceled', () => {
cy.visit('http://localhost:3500/fixtures/generic.html')
cy.intercept('POST', /users/, {
body: { name: 'b' },
delay: 2000,
}).as('createUser')
cy.window()
.then((win) => {
const controller = new AbortController()
const { signal } = controller
fetch('/users/', { signal, method: 'POST' }).catch((e) => {
// do nothing on an abort
})
// if you abort too fast in firefox or safari, the fetch is never sent to the server for us to intercept
if (!Cypress.isBrowser('chrome')) {
setTimeout(() => {
controller.abort()
}, 100)
} else {
controller.abort()
}
cy.wait('@createUser').its('state').should('eq', 'Errored')
})
})
})
// TODO: fix flaky tests https://github.com/cypress-io/cypress/issues/23434
describe.skip('network stubbing', function () {
const { $, _, sinon, state, Promise } = Cypress
@@ -69,8 +69,7 @@ if (Cypress.isBrowser('chrome')) {
})
cy
.server()
.route('GET', /timeout/).as('getTimeout')
.intercept('GET', /timeout/, { delay: 2000 }).as('getTimeout')
.visit('http://localhost:3500/fixtures/generic.html')
.window()
.then((win) => {
@@ -107,13 +106,13 @@ if (Cypress.isBrowser('chrome')) {
// after we unload we should cancel the
// pending XHR's and receive it here
// after waiting on it
expect(xhrProxy.canceled).to.be.true
expect(xhrProxy.state).to.eq('Errored')
const [firstXhr, secondXhr] = xhrs
const [firstLog, secondLog] = logs
// should be the same XHR here as the proxy's XHR
expect(secondXhr === xhrProxy.xhr).to.be.true
expect(secondXhr.url === xhrProxy.request.url).to.be.true
expect(firstXhr.canceled).not.to.be.true
expect(firstXhr.aborted).not.to.be.true
@@ -38,7 +38,7 @@ export const onNetworkError: HandlerFn<CyHttpMessages.NetworkError> = async (Cyp
if (isAwaitingResponse) {
// the user is implicitly expecting there to be a successful response from the server, so fail the test
// since a network error has occured
// since a network error has occurred
throw err
}
@@ -92,8 +92,23 @@ export const InterceptRequest: RequestMiddleware = async function () {
return resolve()
}
const onClose = (): void => {
req.body = ''
return resolve()
}
// If the response has been destroyed we won't be able to get the body from the stream.
if (request.res.destroyed) {
onClose()
}
// Also listen the response close in case it happens while we are piping the request stream.
request.res.once('close', onClose)
request.req.pipe(concatStream((reqBody) => {
req.body = reqBody
request.res.off('close', onClose)
resolve()
}))
})
+40 -15
View File
@@ -107,10 +107,10 @@ export type HttpMiddlewareThis<T> = HttpMiddlewareCtx<T> & ServerCtx & Readonly<
skipMiddleware: (name: string) => void
}>
export function _runStage (type: HttpStages, ctx: any, onError) {
export function _runStage (type: HttpStages, ctx: any, onError: Function) {
ctx.stage = HttpStages[type]
const runMiddlewareStack = () => {
const runMiddlewareStack = (): Promise<void> => {
const middlewares = ctx.middleware[type]
// pop the first pair off the middleware
@@ -138,7 +138,32 @@ export function _runStage (type: HttpStages, ctx: any, onError) {
.value()
}
function _onError (error: Error) {
ctx.debug('Error in middleware %o', { middlewareName, error })
if (type === HttpStages.Error) {
return
}
ctx.res.off('close', onClose)
_end(onError(error))
}
function onClose () {
if (!ctx.res.writableFinished) {
_onError(new Error('Socket closed before finished writing response.'))
}
}
// If we are in the middle of the response phase we want to listen for the on close message and abort responding and instead send an error.
// If the response is closed before the middleware completes, it implies the that request was canceled by the browser.
// The request phase is handled elsewhere because we always want the request phase to complete before erroring on canceled.
if (type === HttpStages.IncomingResponse) {
ctx.res.on('close', onClose)
}
function _end (retval?) {
ctx.res.off('close', onClose)
if (ended) {
return
}
@@ -162,26 +187,17 @@ export function _runStage (type: HttpStages, ctx: any, onError) {
copyChangedCtx()
ctx.res.off('close', onClose)
_end(runMiddlewareStack())
},
end: () => _end(),
end: _end,
onResponse: (incomingRes: Response, resStream: Readable) => {
ctx.incomingRes = incomingRes
ctx.incomingResStream = resStream
_end()
},
onError: (error: Error) => {
ctx.debug('Error in middleware %o', { middlewareName, error })
if (type === HttpStages.Error) {
return
}
ctx.error = error
onError(error)
_end(_runStage(HttpStages.Error, ctx, onError))
},
onError: _onError,
skipMiddleware: (name) => {
ctx.middleware[type] = _.omit(ctx.middleware[type], name)
},
@@ -289,7 +305,8 @@ export class Http {
},
}
const onError = () => {
const onError = (error: Error): Promise<void> => {
ctx.error = error
if (ctx.req.browserPreRequest) {
// browsers will retry requests in the event of network errors, but they will not send pre-requests,
// so try to re-use the current browserPreRequest for the next retry after incrementing the ID.
@@ -301,10 +318,18 @@ export class Http {
ctx.debug('Re-using pre-request data %o', preRequest)
this.addPendingBrowserPreRequest(preRequest)
}
return _runStage(HttpStages.Error, ctx, onError)
}
return _runStage(HttpStages.IncomingRequest, ctx, onError)
.then(() => {
// If the response has been destroyed after handling the incoming request, it implies the that request was canceled by the browser.
// In this case we don't want to run the response middleware and should just exit.
if (res.destroyed) {
return onError(new Error('Socket closed before finished writing response'))
}
if (ctx.incomingRes) {
return _runStage(HttpStages.IncomingResponse, ctx, onError)
}
@@ -28,6 +28,10 @@ describe('http/error-middleware', function () {
outgoingReq: {
abort: sinon.stub(),
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
return testMiddleware([AbortRequest], ctx)
@@ -37,7 +41,12 @@ describe('http/error-middleware', function () {
})
it('does not destroy outgoingReq if it does not exist', function () {
return testMiddleware([AbortRequest], {})
return testMiddleware([AbortRequest], {
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
})
})
})
@@ -47,6 +56,10 @@ describe('http/error-middleware', function () {
incomingResStream: {
unpipe: sinon.stub(),
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
return testMiddleware([UnpipeResponse], ctx)
@@ -56,7 +69,12 @@ describe('http/error-middleware', function () {
})
it('does not unpipe incomingRes if it does not exist', function () {
return testMiddleware([UnpipeResponse], {})
return testMiddleware([UnpipeResponse], {
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
})
})
})
@@ -65,6 +83,8 @@ describe('http/error-middleware', function () {
const ctx = {
res: {
destroy: sinon.stub(),
on: (event, listener) => {},
off: (event, listener) => {},
},
}
+17 -4
View File
@@ -10,12 +10,16 @@ describe('http', function () {
let incomingResponse
let error
let httpOpts
let on
let off
beforeEach(function () {
config = {}
incomingRequest = sinon.stub()
incomingResponse = sinon.stub()
error = sinon.stub()
on = sinon.stub()
off = sinon.stub()
middleware = {
[HttpStages.IncomingRequest]: [incomingRequest],
@@ -45,11 +49,13 @@ describe('http', function () {
return new Http(httpOpts)
// @ts-ignore
.handle({}, {})
.handle({}, { on, off })
.then(function () {
expect(incomingRequest, 'incomingRequest').to.be.calledOnce
expect(incomingResponse, 'incomingResponse').to.be.calledOnce
expect(error).to.not.be.called
expect(on).to.be.calledOnce
expect(off).to.be.calledTwice
})
})
@@ -63,11 +69,13 @@ describe('http', function () {
return new Http(httpOpts)
// @ts-ignore
.handle({}, {})
.handle({}, { on, off })
.then(function () {
expect(incomingRequest).to.be.calledOnce
expect(incomingResponse).to.not.be.called
expect(error).to.be.calledOnce
expect(on).to.not.be.called
expect(off).to.be.calledThrice
})
})
@@ -86,11 +94,13 @@ describe('http', function () {
return new Http(httpOpts)
// @ts-ignore
.handle({}, {})
.handle({}, { on, off })
.then(function () {
expect(incomingRequest).to.be.calledOnce
expect(incomingResponse).to.be.calledOnce
expect(error).to.be.calledOnce
expect(on).to.be.calledOnce
expect(off).to.have.callCount(4)
})
})
@@ -149,7 +159,7 @@ describe('http', function () {
return new Http(httpOpts)
// @ts-ignore
.handle({}, {})
.handle({}, { on, off })
.then(function () {
[
incomingRequest, incomingRequest2,
@@ -158,6 +168,9 @@ describe('http', function () {
].forEach(function (fn) {
expect(fn).to.be.calledOnce
})
expect(on).to.be.calledTwice
expect(off).to.have.callCount(10)
})
})
})
@@ -38,6 +38,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-aut-frame': 'true',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -52,6 +56,10 @@ describe('http/request-middleware', () => {
req: {
headers: {},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -68,6 +76,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'true',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -81,6 +93,10 @@ describe('http/request-middleware', () => {
req: {
headers: {},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -99,6 +115,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'true',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -119,6 +139,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'true',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -142,6 +166,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'sub_frame',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -170,6 +198,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'xhr',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -197,6 +229,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'fetch',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -226,6 +262,10 @@ describe('http/request-middleware', () => {
'x-cypress-is-xhr-or-fetch': 'true',
},
} as Partial<CypressIncomingRequest>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([ExtractCypressMetadataHeaders], ctx)
@@ -249,6 +289,10 @@ describe('http/request-middleware', () => {
'sec-fetch-dest': 'iframe',
},
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([MaybeSimulateSecHeaders], ctx)
@@ -267,6 +311,10 @@ describe('http/request-middleware', () => {
'sec-fetch-dest': 'iframe',
},
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([MaybeSimulateSecHeaders], ctx)
@@ -285,6 +333,10 @@ describe('http/request-middleware', () => {
'sec-fetch-dest': 'video',
},
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([MaybeSimulateSecHeaders], ctx)
@@ -303,6 +355,10 @@ describe('http/request-middleware', () => {
'sec-fetch-dest': 'iframe',
},
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
}
await testMiddleware([MaybeSimulateSecHeaders], ctx)
@@ -539,6 +595,10 @@ describe('http/request-middleware', () => {
cookie: requestCookieStrings.join('; ') || undefined,
},
},
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
} as HttpMiddlewareThis<any>
}
})
@@ -557,7 +617,10 @@ describe('http/request-middleware', () => {
req: {
proxiedUrl: 'https://www.cypress.io/',
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
}
await testMiddleware([MaybeEndRequestWithBufferedResponse], ctx)
@@ -580,7 +643,10 @@ describe('http/request-middleware', () => {
config: {
experimentalSessionAndOrigin: true,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
}
await testMiddleware([MaybeEndRequestWithBufferedResponse], ctx)
@@ -603,7 +669,10 @@ describe('http/request-middleware', () => {
config: {
experimentalSessionAndOrigin: false,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
}
await testMiddleware([MaybeEndRequestWithBufferedResponse], ctx)
@@ -623,7 +692,10 @@ describe('http/request-middleware', () => {
req: {
proxiedUrl: 'https://www.not-cypress.io/',
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
}
await testMiddleware([MaybeEndRequestWithBufferedResponse], ctx)
@@ -647,7 +719,10 @@ describe('http/request-middleware', () => {
proxiedUrl: 'https://www.cypress.io/',
headers,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
remoteStates,
}
@@ -670,7 +745,10 @@ describe('http/request-middleware', () => {
proxiedUrl: 'https://www.cypress.io/',
headers,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
remoteStates,
}
@@ -691,7 +769,10 @@ describe('http/request-middleware', () => {
proxiedUrl: 'https://www.cypress.io/',
headers,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
remoteStates,
}
@@ -712,7 +793,10 @@ describe('http/request-middleware', () => {
proxiedUrl: 'https://www.cypress.io/',
headers,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
remoteStates,
}
@@ -735,7 +819,10 @@ describe('http/request-middleware', () => {
proxiedUrl: 'https://www.cypress.io/',
headers,
},
res: {} as Partial<CypressOutgoingResponse>,
res: {
on: (event, listener) => {},
off: (event, listener) => {},
} as Partial<CypressOutgoingResponse>,
remoteStates,
}
@@ -38,6 +38,10 @@ describe('http/response-middleware', function () {
}
testMiddleware([middleware], {
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
onError (err) {
expect(err.message).to.equal('Error running proxy middleware: Cannot call this.next() more than once in the same middleware function. Doing so can cause unintended issues.')
@@ -55,6 +59,10 @@ describe('http/response-middleware', function () {
}
return testMiddleware([middleware1, middleware2], {
res: {
on: (event, listener) => {},
off: (event, listener) => {},
},
onError () {
throw new Error('onError should not be called')
},
@@ -152,6 +160,8 @@ describe('http/response-middleware', function () {
res: {
set: sinon.stub(),
removeHeader: sinon.stub(),
on: (event, listener) => {},
off: (event, listener) => {},
},
incomingRes: {
headers,
@@ -593,6 +603,8 @@ describe('http/response-middleware', function () {
res: {
headers: {},
setHeader: sinon.stub(),
on: (event, listener) => {},
off: (event, listener) => {},
...props.res,
},
req: {
@@ -1387,7 +1399,8 @@ describe('http/response-middleware', function () {
},
res: {
headers: {},
on () {},
on: (event, listener) => {},
off: (event, listener) => {},
...props.res,
},
req: {
@@ -1558,6 +1571,8 @@ describe('http/response-middleware', function () {
res: {
wantsInjection: 'full',
wantsSecurityRemoved: true,
on: (event, listener) => {},
off: (event, listener) => {},
...props.res,
},
req: {
@@ -1673,6 +1688,8 @@ describe('http/response-middleware', function () {
res: {
wantsInjection: 'full',
wantsSecurityRemoved: true,
on: (event, listener) => {},
off: (event, listener) => {},
...props.res,
},
req: {
@@ -129,11 +129,8 @@ describe('xhrs', () => {
})
describe('server with 1 visit', () => {
before(() => {
cy.visit('/xhr.html')
})
beforeEach(() => {
cy.visit('/xhr.html')
cy.server()
cy.route(/users/, [{}, {}]).as('getUsers')
})
@@ -157,10 +154,8 @@ describe('xhrs', () => {
it('aborts', () => {
cy.window()
.then((win) => {
cy.route({
method: 'POST',
url: /users/,
response: { name: 'b' },
cy.intercept('POST', /users/, {
body: { name: 'b' },
delay: 2000,
}).as('createUser')
@@ -168,7 +163,7 @@ describe('xhrs', () => {
return win.location.href = '/index.html'
})
cy.wait('@createUser').its('canceled').should('be.true')
cy.wait('@createUser').its('state').should('eq', 'Errored')
})
})
})