Fix cy.visit slowness by removing Electron timers workaround

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

@@ -577,15 +577,19 @@ module.exports = (Commands, Cypress, cy, state, config) ->
 
           $utils.iframeSrc($autIframe, url)
 
-      onLoad = ({runOnLoadCallback}) ->
+      onLoad = ({runOnLoadCallback, totalTime}) ->
         ## reset window on load
         win = state("window")
 
         ## the onLoad callback should only be skipped if specified
         if runOnLoadCallback isnt false
           options.onLoad?.call(runnable.ctx, win)
 
-        options._log.set({url: url}) if options._log
+        if options._log
+          options._log.set({
+            url
+            totalTime
+          })
 
         return Promise.resolve(win)
 
@@ -680,7 +684,8 @@ module.exports = (Commands, Cypress, cy, state, config) ->
             url = $Location.fullyQualifyUrl(url)
 
             changeIframeSrc(url, "window:load")
-            .then(onLoad)
+            .then ->
+              onLoad(resp)
           else
             ## if we've already visited a new superDomain
             ## then die else we'd be in a terrible endless loop
@@ -784,8 +789,6 @@ module.exports = (Commands, Cypress, cy, state, config) ->
           go()
 
       visit()
-      .then ->
-        state("window")
       .timeout(options.timeout, "visit")
       .catch Promise.TimeoutError, (err) =>
         timedOutWaitingForPageLoad(options.timeout, options._log)

packages/network/lib/agent.ts

@@ -138,7 +138,18 @@ export class CombinedAgent {
   }
 
   // called by Node.js whenever a new request is made internally
-  addRequest(req: http.ClientRequest, options: http.RequestOptions) {
+  addRequest(req: http.ClientRequest, options: http.RequestOptions, port?: number, localAddress?: string) {
+    // Legacy API: addRequest(req, host, port, localAddress)
+    // https://github.com/nodejs/node/blob/cb68c04ce1bc4534b2d92bc7319c6ff6dda0180d/lib/_http_agent.js#L148-L155
+    if (typeof options === 'string') {
+      // @ts-ignore
+      options = {
+        host: options,
+        port: port!,
+        localAddress
+      }
+    }
+
     const isHttps = isRequestHttps(options)
 
     if (!options.href) {
@@ -156,11 +167,13 @@ export class CombinedAgent {
       }
     }
 
-    debug(`addRequest called for ${options.href}`)
+    debug('addRequest called %o', { isHttps, ..._.pick(options, 'href') })
 
     return getFirstWorkingFamily(options, this.familyCache, (family: net.family) => {
       options.family = family
 
+      debug('got family %o', _.pick(options, 'family', 'href'))
+
       if (isHttps) {
         return this.httpsAgent.addRequest(req, options)
       }
@@ -180,22 +193,22 @@ class HttpAgent extends http.Agent {
     this.httpsAgent = new https.Agent({ keepAlive: true })
   }
 
-  createSocket (req: http.ClientRequest, options: http.RequestOptions, cb: http.SocketCallback) {
+  addRequest (req: http.ClientRequest, options: http.RequestOptions) {
     if (process.env.HTTP_PROXY) {
       const proxy = getProxyForUrl(options.href)
 
       if (proxy) {
         options.proxy = proxy
 
-        return this._createProxiedSocket(req, <RequestOptionsWithProxy>options, cb)
+        return this._addProxiedRequest(req, <RequestOptionsWithProxy>options)
       }
     }
 
-    super.createSocket(req, options, cb)
+    super.addRequest(req, options)
   }
 
-  _createProxiedSocket (req: http.ClientRequest, options: RequestOptionsWithProxy, cb: http.SocketCallback) {
-    debug(`Creating proxied socket for ${options.href} through ${options.proxy}`)
+  _addProxiedRequest (req: http.ClientRequest, options: RequestOptionsWithProxy) {
+    debug(`Creating proxied request for ${options.href} through ${options.proxy}`)
 
     const proxy = url.parse(options.proxy)
 
@@ -226,7 +239,7 @@ class HttpAgent extends http.Agent {
       return this.httpsAgent.addRequest(req, options)
     }
 
-    super.createSocket(req, options, cb)
+    super.addRequest(req, options)
   }
 }
 

packages/network/lib/connect.ts

@@ -22,6 +22,8 @@ export function byPortAndAddress (port: number, address: net.Address) {
 }
 
 export function getAddress (port: number, hostname: string) {
+  debug('beginning getAddress %o', { hostname, port })
+
   const fn = byPortAndAddress.bind({}, port)
 
   // promisify at the very last second which enables us to
@@ -35,9 +37,13 @@ export function getAddress (port: number, hostname: string) {
   // @ts-ignore
   return lookupAsync(hostname, { all: true })
   .then((addresses: net.Address[]) => {
+    debug('got addresses %o', { hostname, port, addresses })
     // convert to an array if string
     return Array.prototype.concat.call(addresses).map(fn)
   })
+  .tapCatch((err) => {
+    debug('error getting address', { hostname, port, err })
+  })
   .any()
 }
 

packages/network/test/integration/connect_spec.ts

@@ -0,0 +1,53 @@
+import _ from 'lodash'
+import Bluebird from 'bluebird'
+import chai from 'chai'
+import net from 'net'
+import sinonChai from 'sinon-chai'
+import * as connect from '../../lib/connect'
+
+const expect = chai.expect
+chai.use(sinonChai)
+
+describe('lib/connect', function() {
+  context('.getAddress', function() {
+    it('resolves localhost on 127.0.0.1 immediately', function() {
+      this.timeout(50)
+
+      const server = net.createServer(_.partialRight(_.invoke, 'close'))
+
+      return Bluebird.fromCallback(cb => {
+        server.listen(0, '127.0.0.1', cb)
+      })
+      .then(() => {
+        return connect.getAddress(server.address().port, 'localhost')
+      })
+      .then((address) => {
+        expect(address).to.deep.eq({
+          family: 4,
+          address: '127.0.0.1'
+        })
+      })
+    })
+
+    // Error: listen EADDRNOTAVAIL ::1
+    // TODO: add an ipv6 lo if to the docker container
+    it.skip('resolves localhost on ::1 immediately', function() {
+      this.timeout(50)
+
+      const server = net.createServer(_.partialRight(_.invoke, 'close'))
+
+      return Bluebird.fromCallback(cb => {
+        server.listen(0, '::1', cb)
+      })
+      .then(() => {
+        return connect.getAddress(server.address().port, 'localhost')
+      })
+      .then((address) => {
+        expect(address).to.deep.eq({
+          family: 6,
+          address: '::1'
+        })
+      })
+    })
+  })
+})

packages/network/test/mocha.opts

@@ -1,4 +1,5 @@
 test/unit
+test/integration
 --compilers ts:@packages/ts/register
 --timeout 10000
 --recursive

packages/network/test/unit/agent_spec.ts

@@ -1,6 +1,5 @@
 import Bluebird from 'bluebird'
 import chai from 'chai'
-import { EventEmitter } from 'events'
 import http from 'http'
 import https from 'https'
 import net from 'net'

packages/server/__snapshots__/6_visit_spec.coffee.js

@@ -819,3 +819,123 @@ Error: ESOCKETTIMEDOUT
 
 
 `
+
+exports['e2e visit resolves visits quickly in chrome (headed) 1'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (fast_visit_spec.coffee)                                                   │
+  │ Searched:   cypress/integration/fast_visit_spec.coffee                                         │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+
+  Running: fast_visit_spec.coffee...                                                       (1 of 1) 
+
+Warning: Cypress can only record videos when using the built in 'electron' browser.
+
+You have set the browser to: 'chrome'
+
+A video will not be recorded when using this browser.
+
+
+  ✓ normally finishes in less than  XX:XX on localhost with connection: close
+  ✓ normally finishes in less than  XX:XX on localhost with connection: keep-alive
+
+  2 passing
+
+
+  (Results)
+
+  ┌──────────────────────────────────────┐
+  │ Tests:        2                      │
+  │ Passing:      2                      │
+  │ Failing:      0                      │
+  │ Pending:      0                      │
+  │ Skipped:      0                      │
+  │ Screenshots:  0                      │
+  │ Video:        false                  │
+  │ Duration:     X seconds              │
+  │ Spec Ran:     fast_visit_spec.coffee │
+  └──────────────────────────────────────┘
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+      Spec                                                Tests  Passing  Failing  Pending  Skipped 
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔ fast_visit_spec.coffee                    XX:XX        2        2        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    All specs passed!                           XX:XX        2        2        -        -        -  
+
+
+`
+
+exports['e2e visit resolves visits quickly in electron (headless) 1'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (fast_visit_spec.coffee)                                                   │
+  │ Searched:   cypress/integration/fast_visit_spec.coffee                                         │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+
+  Running: fast_visit_spec.coffee...                                                       (1 of 1) 
+
+
+  ✓ normally finishes in less than  XX:XX on localhost with connection: close
+  ✓ normally finishes in less than  XX:XX on localhost with connection: keep-alive
+
+  2 passing
+
+
+  (Results)
+
+  ┌──────────────────────────────────────┐
+  │ Tests:        2                      │
+  │ Passing:      2                      │
+  │ Failing:      0                      │
+  │ Pending:      0                      │
+  │ Skipped:      0                      │
+  │ Screenshots:  0                      │
+  │ Video:        true                   │
+  │ Duration:     X seconds              │
+  │ Spec Ran:     fast_visit_spec.coffee │
+  └──────────────────────────────────────┘
+
+
+  (Video)
+
+  - Started processing:   Compressing to 32 CRF
+  - Finished processing:  /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+      Spec                                                Tests  Passing  Failing  Pending  Skipped 
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔ fast_visit_spec.coffee                    XX:XX        2        2        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    All specs passed!                           XX:XX        2        2        -        -        -  
+
+
+`

packages/server/index.js

@@ -1,12 +1,6 @@
 // override tty if we're being forced to
 require('./lib/util/tty').override()
 
-// if we are running in electron
-// we must hack around busted timers
-if (process.versions.electron) {
-  require('./timers/parent').fix()
-}
-
 if (process.env.CY_NET_PROFILE && process.env.CYPRESS_ENV) {
   const netProfiler = require('./lib/util/net_profiler')()
 

packages/server/lib/controllers/proxy.coffee

@@ -348,7 +348,7 @@ module.exports = {
     return if res.headersSent
 
     ## omit problematic headers
-    headers = _.omit incomingRes.headers, "set-cookie", "x-frame-options", "content-length", "content-security-policy"
+    headers = _.omit incomingRes.headers, "set-cookie", "x-frame-options", "content-length", "content-security-policy", "connection"
 
     ## do not cache when we inject content into responses
     ## later on we should switch to an etag system so we dont

packages/server/lib/plugins/child/index.js

@@ -1,8 +1,3 @@
-// if we are running in electron
-// we must hack around busted timers
-if (process.versions.electron) {
-  require('../../timers/parent').fix()
-}
 require('graceful-fs').gracefulify(require('fs'))
 require('@packages/coffee/register')
 require && require.extensions && delete require.extensions['.litcoffee']

packages/server/lib/server.coffee

@@ -317,6 +317,8 @@ class Server
       options
     })
 
+    startTime = new Date()
+
     ## if we have an existing url resolver
     ## in flight then cancel it
     if @_urlResolver
@@ -452,6 +454,8 @@ class Server
                 ## connection hangs before receiving a body.
                 debug("resolve:url response ended, setting buffer %o", { newUrl, details })
 
+                details.totalTime = new Date() - startTime
+
                 ## TODO: think about moving this logic back into the
                 ## frontend so that the driver can be in control of
                 ## when the server should cache the request buffer