rework failurs in server and remove bluebird usage from network

Author: AtofStryker

packages/network/lib/agent.ts

@@ -150,7 +150,7 @@ export const regenerateRequestHead = (req: http.ClientRequest) => {
   }
 }
 
-export const getFirstWorkingFamily = (
+export const getFirstWorkingFamily = async (
   { port, host }: Pick<http.RequestOptions, 'port' | 'host'>,
   familyCache: FamilyCache,
   cb: (family?: net.family) => void,
@@ -178,15 +178,15 @@ export const getFirstWorkingFamily = (
     return cb(familyCache[cacheKey])
   }
 
-  return getAddress(port, host)
-  .then((firstWorkingAddress: net.Address) => {
+  try {
+    const firstWorkingAddress = await getAddress(port, host)
+
     familyCache[cacheKey] = firstWorkingAddress.family
 
     return cb(firstWorkingAddress.family)
-  })
-  .catch(() => {
+  } catch (error) {
     return cb()
-  })
+  }
 }
 
 export class CombinedAgent {

packages/network/lib/connect.ts

@@ -1,6 +1,6 @@
-import Bluebird from 'bluebird'
+import { promisify } from 'node:util'
 import debugModule from 'debug'
-import dns, { LookupAddress, LookupAllOptions } from 'dns'
+import dns from 'dns'
 import _ from 'lodash'
 import net from 'net'
 import tls from 'tls'
@@ -27,7 +27,7 @@ export async function getAddress (port: number, hostname: string): Promise<net.A
 
   // promisify at the very last second which enables us to
   // modify dns lookup function (via hosts overrides)
-  const lookupAsync = Bluebird.promisify<LookupAddress[], string, LookupAllOptions>(dns.lookup, { context: dns })
+  const lookupAsync = promisify(dns.lookup)
 
   // this does not go out to the network to figure
   // out the addresses. in fact it respects the /etc/hosts file

packages/network/package.json

@@ -22,7 +22,6 @@
   },
   "dependencies": {
     "@cypress/parse-domain": "2.4.0",
-    "bluebird": "3.5.3",
     "concat-stream": "1.6.2",
     "debug": "^4.3.4",
     "fs-extra": "9.1.0",

packages/network/test/support/servers.ts

@@ -1,4 +1,4 @@
-import Bluebird from 'bluebird'
+import { promisify } from 'util'
 import express from 'express'
 import http from 'http'
 import https from 'https'
@@ -58,17 +58,25 @@ export class Servers {
       getCAInformation(),
     ])
 
-    this.httpServer = Bluebird.promisifyAll(
-      allowDestroy(http.createServer(app)),
-    ) as http.Server & AsyncServer
+    const httpServer = allowDestroy(http.createServer(app))
+
+    this.httpServer = Object.assign(httpServer, {
+      closeAsync: promisify(httpServer.close.bind(httpServer)),
+      destroyAsync: promisify(httpServer.destroy.bind(httpServer)),
+      listenAsync: promisify(httpServer.listen.bind(httpServer)),
+    }) as http.Server & AsyncServer
 
     this.wsServer = new SocketIOServer(this.httpServer)
 
     this.caCertificatePath = caCertificatePath
     this.https = { cert: serverCertificateKeys[0], key: serverCertificateKeys[1] }
-    this.httpsServer = Bluebird.promisifyAll(
-      allowDestroy(https.createServer(this.https, <http.RequestListener>app)),
-    ) as https.Server & AsyncServer
+    const httpsServer = allowDestroy(https.createServer(this.https, <http.RequestListener>app))
+
+    this.httpsServer = Object.assign(httpsServer, {
+      closeAsync: promisify(httpsServer.close.bind(httpsServer)),
+      destroyAsync: promisify(httpsServer.destroy.bind(httpsServer)),
+      listenAsync: promisify(httpsServer.listen.bind(httpsServer)),
+    }) as https.Server & AsyncServer
 
     this.wssServer = new SocketIOServer(this.httpsServer)
 

packages/network/test/unit/agent.spec.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, afterEach, beforeAll, afterAll, vi } from 'vitest'
-import Bluebird from 'bluebird'
+import { promisify } from 'util'
 
 import http from 'http'
 import https from 'https'
@@ -451,13 +451,15 @@ describe('lib/agent', function () {
       })
 
       it('#createUpstreamProxyConnection throws when connection is accepted then closed', async () => {
-        const proxy = Bluebird.promisifyAll(
-          allowDestroy(
-            net.createServer((socket) => {
-              socket.end()
-            }),
-          ),
-        ) as net.Server & AsyncServer
+        const proxyServer = allowDestroy(
+          net.createServer((socket) => {
+            socket.end()
+          }),
+        )
+        const proxy = Object.assign(proxyServer, {
+          destroyAsync: promisify(proxyServer.destroy.bind(proxyServer)),
+          listenAsync: promisify(proxyServer.listen.bind(proxyServer)),
+        }) as net.Server & AsyncServer
 
         const proxyPort = PROXY_PORT + 2
 

packages/server/lib/util/ensure-url.ts

@@ -70,5 +70,12 @@ export const isListening = (urlStr: string) => {
     .catch({ name: 'StatusCodeError' }, () => {}) // we just care if it can connect, not if it's a valid resource
   }
 
-  return connect.getAddress(Number(port), String(hostname))
+  // With https://github.com/cypress-io/cypress/pull/32633, the @packages/network package has refactored some methods to use
+  // native promises. This method wraps the native promise in a Bluebird promise to ensure that the method returns a Bluebird promise
+  // until we are able to refactor it.
+  return new Bluebird((resolve, reject) => {
+    connect.getAddress(Number(port), String(hostname))
+    .then(resolve)
+    .catch(reject)
+  })
 }

packages/server/test/unit/cloud/api/api_spec.js

@@ -12,6 +12,7 @@ const encryption = require('../../../../lib/cloud/encryption')
 const {
   agent,
 } = require('@packages/network')
+const { CombinedAgent } = require('@packages/network/lib/agent')
 const pkg = require('@packages/root')
 const api = require('../../../../lib/cloud/api').default
 const cache = require('../../../../lib/cache').cache
@@ -140,7 +141,10 @@ describe('lib/cloud/api', () => {
 
   context('.rp', () => {
     beforeEach(() => {
-      sinon.spy(agent, 'addRequest')
+      // Because @packages/network is a bundled ES6 class with https://github.com/cypress-io/cypress/pull/32633
+      // we can no longer mock the addRequest method on the agent singleton instance directly
+      // Until we are able to convert this test to Vitest/TypeScript, we need to spy on the prototype
+      sinon.spy(CombinedAgent.prototype, 'addRequest')
 
       return nock.enableNetConnect()
     }) // nock will prevent requests from reaching the agent
@@ -151,9 +155,9 @@ describe('lib/cloud/api', () => {
       return api.ping()
       .thenThrow()
       .catch(() => {
-        expect(agent.addRequest).to.be.calledOnce
+        expect(CombinedAgent.prototype.addRequest).to.be.calledOnce
 
-        expect(agent.addRequest).to.be.calledWithMatch(sinon.match.any, {
+        expect(CombinedAgent.prototype.addRequest).to.be.calledWithMatch(sinon.match.any, {
           href: 'http://localhost:1234/ping',
         })
       })
@@ -165,9 +169,9 @@ describe('lib/cloud/api', () => {
       return api.ping()
       .thenThrow()
       .catch(() => {
-        expect(agent.addRequest).to.be.calledOnce
+        expect(CombinedAgent.prototype.addRequest).to.be.calledOnce
 
-        expect(agent.addRequest).to.be.calledWithMatch(sinon.match.any, {
+        expect(CombinedAgent.prototype.addRequest).to.be.calledWithMatch(sinon.match.any, {
           rejectUnauthorized: true,
         })
       })
@@ -185,9 +189,9 @@ describe('lib/cloud/api', () => {
         return api.ping()
         .thenThrow()
         .catch(() => {
-          expect(agent.addRequest).to.be.calledOnce
+          expect(CombinedAgent.prototype.addRequest).to.be.calledOnce
 
-          expect(agent.addRequest).to.be.calledWithMatch(sinon.match.any, {
+          expect(CombinedAgent.prototype.addRequest).to.be.calledWithMatch(sinon.match.any, {
             href: 'http://localhost:1234/ping',
           })
         })