fix: better ip version detection when using skip wait port property in dev command (#7074)

Author: VitaliyR

src/utils/proxy.ts

@@ -4,10 +4,12 @@ import { readFile } from 'fs/promises'
 import http, { ServerResponse } from 'http'
 import https from 'https'
 import { isIPv6 } from 'net'
+import { Socket } from 'node:net'
 import { Readable } from 'node:stream'
 import path from 'path'
 import process from 'process'
 import { Duplex } from 'stream'
+import url from 'url'
 import util from 'util'
 import zlib from 'zlib'
 
@@ -20,9 +22,9 @@ import httpProxy from 'http-proxy'
 import { createProxyMiddleware } from 'http-proxy-middleware'
 import { jwtDecode } from 'jwt-decode'
 import { locatePath } from 'locate-path'
+import throttle from 'lodash/throttle.js'
 import { Match } from 'netlify-redirector'
 import pFilter from 'p-filter'
-import throttle from 'lodash/throttle.js'
 
 import { BaseCommand } from '../commands/index.js'
 import { $TSFixMe, NetlifyOptions } from '../commands/types.js'
@@ -515,18 +517,41 @@ const initializeProxy = async function ({
     }
   })
 
-  proxy.on('error', (_, req, res) => {
-    // @ts-expect-error TS(2339) FIXME: Property 'writeHead' does not exist on type 'Socke... Remove this comment to see the full error message
-    res.writeHead(500, {
-      'Content-Type': 'text/plain',
-    })
+  proxy.on('error', (err, req, res, proxyUrl) => {
+    // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
+    const options = req.proxyOptions
+
+    const isConRefused = 'code' in err && err.code === 'ECONNREFUSED'
+    if (options?.detectTarget && !(res instanceof Socket) && isConRefused && proxyUrl) {
+      // got econnrefused while detectTarget set to true -> try to switch between current ipVer and other (4 to 6 and vice versa)
+
+      // proxyUrl is parsed in http-proxy using url, parsing the same here. Difference between it and
+      // URL that hostname not includes [] symbols when using url.parse
+      // eslint-disable-next-line n/no-deprecated-api
+      const targetUrl = typeof proxyUrl === 'string' ? url.parse(proxyUrl) : proxyUrl
+      const isCurrentHost = targetUrl.hostname === options.targetHostname
+      if (targetUrl.hostname && isCurrentHost) {
+        const newHost = isIPv6(targetUrl.hostname) ? '127.0.0.1' : '::1'
+        options.target = `http://${isIPv6(newHost) ? `[${newHost}]` : newHost}:${targetUrl.port}`
+        options.targetHostname = newHost
+        options.isChangingTarget = true
+        return proxy.web(req, res, options)
+      }
+    }
+
+    if (res instanceof http.ServerResponse) {
+      res.writeHead(500, {
+        'Content-Type': 'text/plain',
+      })
+    }
 
     const message = isEdgeFunctionsRequest(req)
       ? 'There was an error with an Edge Function. Please check the terminal for more details.'
       : 'Could not proxy request.'
 
     res.end(message)
   })
+
   proxy.on('proxyReq', (proxyReq, req) => {
     const requestID = generateRequestID()
 
@@ -548,6 +573,7 @@ const initializeProxy = async function ({
       proxyReq.write(req.originalBody)
     }
   })
+
   proxy.on('proxyRes', (proxyRes, req, res) => {
     res.setHeader('server', 'Netlify')
 
@@ -557,6 +583,23 @@ const initializeProxy = async function ({
       res.setHeader(NFRequestID, requestID)
     }
 
+    // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
+    const options = req.proxyOptions
+
+    if (options.isChangingTarget) {
+      // got a response after switching the ipVer for host (and its not an error since we will be in on('error') handler) - let's remember this host now
+
+      // options are not exported in ts for the proxy:
+      // @ts-expect-error TS(2339) FIXME: Property 'options' does not exist on type 'In...
+      proxy.options.target.host = options.targetHostname
+
+      options.changeSettings?.({
+        frameworkHost: options.targetHostname,
+        detectFrameworkHost: false,
+      })
+      console.log(`${NETLIFYDEVLOG} Switched host to ${options.targetHostname}`)
+    }
+
     if (proxyRes.statusCode === 404 || proxyRes.statusCode === 403) {
       // If a request for `/path` has failed, we'll a few variations like
       // `/path/index.html` to mimic the CDN behavior.
@@ -571,8 +614,7 @@ const initializeProxy = async function ({
       // The request has failed but we might still have a matching redirect
       // rule (without `force`) that should kick in. This is how we mimic the
       // file shadowing behavior from the CDN.
-      // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-      if (req.proxyOptions && req.proxyOptions.match) {
+      if (options && options.match) {
         return serveRedirect({
           // We don't want to match functions at this point because any redirects
           // to functions will have already been processed, so we don't supply a
@@ -582,18 +624,15 @@ const initializeProxy = async function ({
           res,
           proxy: handlers,
           imageProxy,
-          // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-          match: req.proxyOptions.match,
-          // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-          options: req.proxyOptions,
+          match: options.match,
+          options,
           siteInfo,
           env,
         })
       }
     }
 
-    // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-    if (req.proxyOptions.staticFile && isRedirect({ status: proxyRes.statusCode }) && proxyRes.headers.location) {
+    if (options.staticFile && isRedirect({ status: proxyRes.statusCode }) && proxyRes.headers.location) {
       req.url = proxyRes.headers.location
       return serveRedirect({
         // We don't want to match functions at this point because any redirects
@@ -605,8 +644,7 @@ const initializeProxy = async function ({
         proxy: handlers,
         imageProxy,
         match: null,
-        // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-        options: req.proxyOptions,
+        options,
         siteInfo,
         env,
       })
@@ -634,8 +672,7 @@ const initializeProxy = async function ({
         // @ts-expect-error TS(2345) FIXME: Argument of type 'unknown' is not assignable to pa... Remove this comment to see the full error message
         res.setHeader(key, val)
       })
-      // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-      res.writeHead(req.proxyOptions.status || proxyRes.statusCode, proxyRes.headers)
+      res.writeHead(options.status || proxyRes.statusCode, proxyRes.headers)
 
       proxyRes.on('data', function onData(data) {
         res.write(data)
@@ -656,8 +693,7 @@ const initializeProxy = async function ({
       // @ts-expect-error TS(7005) FIXME: Variable 'responseData' implicitly has an 'any[]' ... Remove this comment to see the full error message
       let responseBody = Buffer.concat(responseData)
 
-      // @ts-expect-error TS(2339) FIXME: Property 'proxyOptions' does not exist on type 'In... Remove this comment to see the full error message
-      let responseStatus = req.proxyOptions.status || proxyRes.statusCode
+      let responseStatus = options.status || proxyRes.statusCode
 
       // `req[shouldGenerateETag]` may contain a function that determines
       // whether the response should have an ETag header.
@@ -805,11 +841,16 @@ const onRequest = async (
     target: `http://${
       settings.frameworkHost && isIPv6(settings.frameworkHost) ? `[${settings.frameworkHost}]` : settings.frameworkHost
     }:${settings.frameworkPort}`,
+    detectTarget: settings.detectFrameworkHost,
+    targetHostname: settings.frameworkHost,
     publicFolder: settings.dist,
     functionsServer,
     functionsPort: settings.functionsPort,
     jwtRolePath: settings.jwtRolePath,
     framework: settings.framework,
+    changeSettings(newSettings: Partial<ServerSettings>) {
+      Object.assign(settings, newSettings)
+    },
   }
 
   if (match) {

src/utils/run-build.ts

@@ -114,6 +114,7 @@ export const runNetlifyBuild = async ({
     })
 
     settings.frameworkHost = ipVersion === 6 ? '::1' : '127.0.0.1'
+    settings.detectFrameworkHost = options.skipWaitPort
   }
 
   if (timeline === 'build') {

src/utils/types.ts

@@ -32,6 +32,8 @@ export type BaseServerSettings = {
   frameworkPort?: number
   /** The host where a proxy can listen to it */
   frameworkHost?: '127.0.0.1' | '::1'
+  /** Try both v4 and v6 IPs */
+  detectFrameworkHost?: boolean
   functions?: string
   /** The framework name ('Create React App') */
   framework?: string

tests/integration/commands/dev/dev.test.ts

@@ -2,6 +2,7 @@
 import fs from 'node:fs/promises'
 import { type AddressInfo } from 'node:net'
 import path from 'node:path'
+import process from 'process'
 
 import jwt, { type JwtPayload } from 'jsonwebtoken'
 import fetch from 'node-fetch'
@@ -397,6 +398,31 @@ describe.concurrent('command/dev', () => {
     })
   })
 
+  test('should detect ipVer when proxying without waiting for port', async (t) => {
+    // ipv6 is default from node 18
+    const nodeVer = Number.parseInt(process.versions.node.split('.')[0])
+    t.expect(nodeVer).toBeGreaterThanOrEqual(18)
+
+    await withSiteBuilder(t, async (builder) => {
+      const externalServer = startExternalServer({
+        host: '127.0.0.1',
+        port: 4567,
+      })
+      await builder.build()
+
+      await withDevServer(
+        { cwd: builder.directory, command: 'node', framework: '#custom', targetPort: 4567, skipWaitPort: true },
+        async (server) => {
+          const response = await fetch(`${server.url}/test`)
+          t.expect(response.status).toBe(200)
+          t.expect(String(server.output)).toContain('Switched host to 127.0.0.1')
+        },
+      )
+
+      externalServer.close()
+    })
+  })
+
   test('should rewrite POST request if content-type is missing and not crash dev server', async (t) => {
     await withSiteBuilder(t, async (builder) => {
       builder.withNetlifyToml({

tests/integration/utils/dev-server.ts

@@ -41,32 +41,37 @@ interface DevServerOptions {
   args?: string[]
   context?: string
   cwd: string
+  framework?: string
+  command?: string
   debug?: boolean
   env?: Record<string, string>
   expectFailure?: boolean
   offline?: boolean
   prompt?: $FIXME[]
   serve?: boolean
   skipWaitPort?: boolean
+  targetPort?: number
 }
 
 // 240 seconds
 const SERVER_START_TIMEOUT = 24e4
 
 const startServer = async ({
   args = [],
+  command,
   context = 'dev',
   cwd,
   debug = false,
   env = {},
   expectFailure = false,
+  framework,
   offline = true,
   prompt,
   serve = false,
   skipWaitPort = false,
+  targetPort,
 }: DevServerOptions): Promise<DevServer | { timeout: boolean; output: string }> => {
   const port = await getPort()
-  const staticPort = await getPort()
   const host = 'localhost'
   const url = `http://${host}:${port}`
 
@@ -77,12 +82,25 @@ const startServer = async ({
     offline ? '--offline' : '',
     '-p',
     port,
-    '--staticServerPort',
-    staticPort,
     debug ? '--debug' : '',
     skipWaitPort ? '--skip-wait-port' : '',
   ]
 
+  if (targetPort) {
+    baseArgs.push('--target-port', targetPort)
+  } else {
+    const staticPort = await getPort()
+    baseArgs.push('--staticServerPort', staticPort)
+  }
+
+  if (framework) {
+    baseArgs.push('--framework', framework)
+  }
+
+  if (command) {
+    baseArgs.push('--command', command)
+  }
+
   // We use `null` to override the default context and actually omit the flag
   // from the command, which is useful in some test scenarios.
   if (context !== null) {

tests/integration/utils/external-server.js

@@ -2,12 +2,12 @@ import { env } from 'process'
 
 import express from 'express'
 
-export const startExternalServer = ({ port } = {}) => {
+export const startExternalServer = ({ host, port } = {}) => {
   const app = express()
   app.use(express.urlencoded({ extended: true }))
   app.all('*', function onRequest(req, res) {
     res.json({ url: req.url, body: req.body, method: req.method, headers: req.headers, env })
   })
 
-  return app.listen({ port })
+  return app.listen({ port, host })
 }