test: cover browser recovery from a stale keep-alive socket

system-tests/projects/e2e/cypress/e2e/network_error_form_retry.cy.js

@@ -0,0 +1,18 @@
+// Separate from the other network error specs because only the Node MITM proxy
+// re-sends the body, so this holds in one proxy mode rather than both.
+describe('network error handling', function () {
+  context('cy.visit() retries', function () {
+    it('re-sends a <form> body on failures', function () {
+      cy.visit({
+        url: '/print-body-third-time-form',
+      })
+      .get('input[type=text]')
+      .type('bar')
+
+      cy.get('input[type=submit]')
+      .click()
+
+      cy.contains('{"foo":"bar"}')
+    })
+  })
+})

system-tests/projects/e2e/cypress/e2e/network_error_handling.cy.js

@@ -20,19 +20,6 @@ describe('network error handling', function () {
       })
       .contains('ok')
     })
-
-    it('re-sends a <form> body on failures', function () {
-      cy.visit({
-        url: '/print-body-third-time-form',
-      })
-      .get('input[type=text]')
-      .type('bar')
-
-      cy.get('input[type=submit]')
-      .click()
-
-      cy.contains('{"foo":"bar"}')
-    })
   })
 
   context('cy.request() retries', function () {

system-tests/projects/e2e/cypress/e2e/network_error_stale_keepalive.cy.js

@@ -0,0 +1,34 @@
+// Recovering a request written to a dead pooled socket is the browser's job now
+// that Cypress no longer replays traffic through Node. Cypress must let it, and
+// must not report the recovered attempt twice.
+const PAIRS = 3
+
+describe('stale keep-alive sockets', function () {
+  it('recovers without surfacing the failed attempt to the test', function () {
+    cy.intercept({ pathname: '/stale-socket' }).as('staleSocket')
+
+    cy.visit('/stale-keepalive.html')
+
+    cy.window().then((win) => {
+      // the warm request leaves a pooled socket behind; the raced one reuses it
+      // and is met with a FIN, the shape of a keep-alive timeout crossing a request
+      const pair = (i) => {
+        return win.fetch(`/stale-socket?warm=${i}`)
+        .then((res) => res.text())
+        .then(() => win.fetch(`/stale-socket?race=${i}`))
+        .then((res) => {
+          expect(res.status, 'the browser retried the dead socket on a new connection').to.eq(200)
+        })
+      }
+
+      return Cypress.Promise.each(Array.from({ length: PAIRS }, (_, i) => i), pair)
+    })
+
+    // a change in connection pooling would otherwise leave this racing nothing
+    cy.request('/stale-socket-stats').its('body.killedOnReusedSocket').should('be.gte', 1)
+
+    // the wire saw more requests than this — a browser-level retry sits below the
+    // layer Cypress intercepts at
+    cy.get('@staleSocket.all').should('have.length', PAIRS * 2)
+  })
+})

system-tests/test/network_error_handling_spec.js

@@ -2,13 +2,9 @@ const _ = require('lodash')
 const express = require('express')
 const path = require('path')
 const debug = require('debug')('cypress:server:network-error-handling-spec')
-const Promise = require('bluebird')
 const bodyParser = require('body-parser')
 const DebugProxy = require('@cypress/debugging-proxy')
-const launcher = require('@packages/launcher')
-const chrome = require('@packages/server/lib/browsers/chrome')
 const systemTests = require('../lib/system-tests').default
-const random = require('@packages/server/lib/util/random')
 const Fixtures = require('../lib/fixtures')
 
 const PORT = 13370
@@ -25,31 +21,9 @@ const getElapsed = () => {
 
 let onVisit = null
 let counts = {}
+let killedOnReusedSocket = 0
 
-const launchBrowser = (url, opts = {}) => {
-  return launcher.detect().then((browsers) => {
-    const browser = _.find(browsers, { name: 'chrome' })
-
-    const args = [
-      `--user-data-dir=/tmp/cy-e2e-${random.id()}`,
-    // headless breaks automatic retries
-    // "--headless"
-    ].concat(
-      chrome._getArgs(browser),
-    ).filter((arg) => {
-      return ![
-        // seems to break chrome's automatic retries
-        '--enable-automation',
-      ].includes(arg)
-    })
-
-    if (opts.withProxy) {
-      args.push(`--proxy-server=http://localhost:${PORT}`)
-    }
-
-    return launcher.launch(browser, url, args)
-  })
-}
+const servedSockets = new WeakSet()
 
 const controllers = {
   loadScriptNetError (req, res) {
@@ -91,26 +65,6 @@ const controllers = {
     return req.socket.destroy()
   },
 
-  afterHeadersReset (req, res) {
-    res.writeHead(200)
-    res.write('')
-
-    return setTimeout(() => {
-      return req.socket.destroy()
-    }
-    , 1000)
-  },
-
-  duringBodyReset (req, res) {
-    res.writeHead(200)
-    res.write('<html>')
-
-    return setTimeout(() => {
-      return req.socket.destroy()
-    }
-    , 1000)
-  },
-
   worksThirdTime (req, res) {
     if (counts[req.url] === 3) {
       return res.send('ok')
@@ -127,24 +81,30 @@ const controllers = {
     return res.sendStatus(500)
   },
 
-  proxyInternalServerError (req, res) {
-    return res.sendStatus(500)
+  load304 (req, res) {
+    return res.type('html').end('<img src="/static/javascript-logo.png"/>')
   },
 
-  proxyBadGateway (req, res) {
-    // https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.5.3
-    // "The server, while acting as a gateway or proxy, received an invalid response"
-    return res.sendStatus(502)
+  staleKeepAlivePage (req, res) {
+    return res.type('html').end('<html><body>stale keep-alive</body></html>')
   },
 
-  proxyServiceUnavailable (req, res) {
-    // https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.5.4
-    // "The implication is that this is a temporary condition which will be alleviated after some delay."
-    return res.sendStatus(503)
+  // A request on an already-served socket is met with a FIN and no response — what
+  // a client sees when the origin's keep-alive timeout crosses a request in flight.
+  staleSocket (req, res) {
+    if (servedSockets.has(req.socket)) {
+      killedOnReusedSocket++
+
+      return req.socket.end()
+    }
+
+    servedSockets.add(req.socket)
+
+    return res.send('ok')
   },
 
-  load304 (req, res) {
-    return res.type('html').end('<img src="/static/javascript-logo.png"/>')
+  staleSocketStats (req, res) {
+    return res.json({ killedOnReusedSocket })
   },
 }
 
@@ -176,34 +136,19 @@ describe('e2e network error handling', function () {
           app.use(bodyParser.urlencoded({ extended: true }))
 
           app.get('/immediate-reset', controllers.immediateReset)
-          app.get('/after-headers-reset', controllers.afterHeadersReset)
-          app.get('/during-body-reset', controllers.duringBodyReset)
           app.get('/works-third-time/:id', controllers.worksThirdTime)
           app.get('/works-third-time-else-500/:id', controllers.worksThirdTimeElse500)
           app.post('/print-body-third-time', controllers.printBodyThirdTime)
 
           app.get('/load-304.html', controllers.load304)
+          app.get('/stale-keepalive.html', controllers.staleKeepAlivePage)
+          app.get('/stale-socket', controllers.staleSocket)
+          app.get('/stale-socket-stats', controllers.staleSocketStats)
           app.get('/load-img-net-error.html', controllers.loadImgNetError)
           app.get('/load-script-net-error.html', controllers.loadScriptNetError)
           app.get('/print-body-third-time-form', controllers.printBodyThirdTimeForm)
 
           return app.get('*', (req, res) => {
-            // pretending we're a http proxy
-            const controller = ({
-              'http://immediate-reset.invalid/': controllers.immediateReset,
-              'http://after-headers-reset.invalid/': controllers.afterHeadersReset,
-              'http://during-body-reset.invalid/': controllers.duringBodyReset,
-              'http://proxy-internal-server-error.invalid/': controllers.proxyInternalServerError,
-              'http://proxy-bad-gateway.invalid/': controllers.proxyBadGateway,
-              'http://proxy-service-unavailable.invalid/': controllers.proxyServiceUnavailable,
-            })[req.url]
-
-            if (controller) {
-              debug('got controller for request')
-
-              return controller(req, res)
-            }
-
             return res.sendStatus(404)
           })
         },
@@ -252,102 +197,6 @@ describe('e2e network error handling', function () {
     counts = {}
   })
 
-  // NOTE: We can just skip these tests, they are really only useful for learning
-  // about how Chrome does it.
-  context.skip('Google Chrome', () => {
-    const testRetries = (path) => {
-      return launchBrowser(`http://127.0.0.1:${PORT}${path}`)
-      .then((proc) => {
-        return Promise.fromCallback((cb) => {
-          return onVisit = function () {
-            if (counts[path] >= 3) {
-              return cb()
-            }
-          }
-        }).then(() => {
-          proc.kill(9)
-
-          expect(counts[path]).to.be.at.least(3)
-        })
-      })
-    }
-
-    const testNoRetries = (path) => {
-      return launchBrowser(`http://localhost:${PORT}${path}`)
-      .delay(6000)
-      .then((proc) => {
-        proc.kill(9)
-
-        expect(counts[path]).to.eq(1)
-      })
-    }
-
-    it('retries 3+ times when receiving immediate reset', () => {
-      return testRetries('/immediate-reset')
-    })
-
-    it('retries 3+ times when receiving reset after headers', () => {
-      return testRetries('/after-headers-reset')
-    })
-
-    it('does not retry if reset during body', () => {
-      return testNoRetries('/during-body-reset')
-    })
-
-    context('behind a proxy server', () => {
-      const testProxiedRetries = (url) => {
-        return launchBrowser(url, { withProxy: true })
-        .then((proc) => {
-          return Promise.fromCallback((cb) => {
-            return onVisit = function () {
-              if (counts[url] >= 3) {
-                return cb()
-              }
-            }
-          }).then(() => {
-            proc.kill(9)
-
-            expect(counts[url]).to.be.at.least(3)
-          })
-        })
-      }
-
-      const testProxiedNoRetries = (url) => {
-        return launchBrowser('http://during-body-reset.invalid/', { withProxy: true })
-        .delay(6000)
-        .then((proc) => {
-          proc.kill(9)
-
-          expect(counts[url]).to.eq(1)
-        })
-      }
-
-      it('retries 3+ times when receiving immediate reset', () => {
-        return testProxiedRetries('http://immediate-reset.invalid/')
-      })
-
-      it('retries 3+ times when receiving reset after headers', () => {
-        return testProxiedRetries('http://after-headers-reset.invalid/')
-      })
-
-      it('does not retry if reset during body', () => {
-        return testProxiedNoRetries('http://during-body-reset.invalid/')
-      })
-
-      it('does not retry on \'500 Internal Server Error\'', () => {
-        return testProxiedNoRetries('http://proxy-internal-server-error.invalid/')
-      })
-
-      it('does not retry on \'502 Bad Gateway\'', () => {
-        return testProxiedNoRetries('http://proxy-bad-gateway.invalid/')
-      })
-
-      it('does not retry on \'503 Service Unavailable\'', () => {
-        return testProxiedNoRetries('http://proxy-service-unavailable.invalid/')
-      })
-    })
-  })
-
   context('Cypress', () => {
     let debugProxy
 
@@ -424,6 +273,49 @@ describe('e2e network error handling', function () {
       })
     })
 
+    it('retries network errors for cy.visit, cy.request, and subresources', function () {
+      return systemTests.exec(this, {
+        spec: 'network_error_handling.cy.js',
+        config: {
+          baseUrl: `http://localhost:${PORT}`,
+        },
+        expectedExitCode: 2,
+      })
+    })
+
+    // NOTE: only the Node hop replays a POST body — the browser will not replay a
+    // non-idempotent request — so this holds only while Cypress proxies.
+    const itReplaysFormBody = process.env.CYPRESS_INTERNAL_DISABLE_PROXY === '1' ? it.skip : it
+
+    itReplaysFormBody('re-sends a <form> body when the origin resets the connection', function () {
+      return systemTests.exec(this, {
+        spec: 'network_error_form_retry.cy.js',
+        config: {
+          baseUrl: `http://localhost:${PORT}`,
+        },
+      })
+    })
+
+    // NOTE: only with the proxy disabled does the browser reach the origin
+    // directly, so this contract exists solely in that mode.
+    const contextStaleKeepAlive = process.env.CYPRESS_INTERNAL_DISABLE_PROXY === '1' ? context : context.skip
+
+    contextStaleKeepAlive('stale keep-alive sockets', () => {
+      beforeEach(() => {
+        killedOnReusedSocket = 0
+      })
+
+      it('lets the browser recover a request written to a dead pooled socket', function () {
+        return systemTests.exec(this, {
+          spec: 'network_error_stale_keepalive.cy.js',
+          browser: 'chrome',
+          config: {
+            baseUrl: `http://localhost:${PORT}`,
+          },
+        })
+      })
+    })
+
     // https://github.com/cypress-io/cypress/issues/4298
     context('does not delay a 304 Not Modified', () => {
       it('in normal network conditions', function () {