fix: support for redirects with 200 status. Fix crashing dev server when have socket error. Follow redirects from proxyRes

Author: VitaliyR

src/utils/proxy.ts

@@ -122,10 +122,14 @@ const formatEdgeFunctionError = (errorBuffer, acceptsHtml) => {
   })
 }
 
-function isInternal(url?: string): boolean {
+function isInternalFunctions(url?: string): boolean {
   return url?.startsWith('/.netlify/') ?? false
 }
 
+function isInternal(url?: string): boolean {
+  return url?.startsWith('/') ?? false
+}
+
 function isFunction(functionsPort: boolean | number | undefined, url: string) {
   return functionsPort && url.match(DEFAULT_FUNCTION_URL_EXPRESSION)
 }
@@ -202,7 +206,7 @@ const handleAddonUrl = function ({ addonUrl, req, res }) {
 
 // @ts-expect-error TS(7006) FIXME: Parameter 'match' implicitly has an 'any' type.
 const isRedirect = function (match) {
-  return match.status && match.status >= 300 && match.status <= 400
+  return match.status && match.status >= 300 && match.status < 400
 }
 
 // @ts-expect-error TS(7006) FIXME: Parameter 'publicFolder' implicitly has an 'any' t... Remove this comment to see the full error message
@@ -417,8 +421,8 @@ const serveRedirect = async function ({
     const ct = req.headers['content-type'] ? contentType.parse(req).type : ''
     if (
       req.method === 'POST' &&
-      !isInternal(req.url) &&
-      !isInternal(destURL) &&
+      !isInternalFunctions(req.url) &&
+      !isInternalFunctions(destURL) &&
       (ct.endsWith('/x-www-form-urlencoded') || ct === 'multipart/form-data')
     ) {
       return proxy.web(req, res, { target: options.functionsServer })
@@ -452,9 +456,11 @@ const serveRedirect = async function ({
 
       return proxy.web(req, res, { headers: functionHeaders, target: options.functionsServer })
     }
+
     if (isImageRequest(req)) {
       return imageProxy(req, res)
     }
+
     const addonUrl = getAddonUrl(options.addonsUrls, req)
     if (addonUrl) {
       return handleAddonUrl({ req, res, addonUrl })
@@ -519,11 +525,14 @@ 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) => {
+    console.error('Got error from proxy', err);
+
+    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.'
@@ -596,24 +605,15 @@ const initializeProxy = async function ({
       }
     }
 
-    // @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) {
-      req.url = proxyRes.headers.location
-      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
-        // functions registry to `serveRedirect`.
-        functionsRegistry: null,
-        req,
-        res,
-        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,
-        siteInfo,
-        env,
+    if (isRedirect({ status: proxyRes.statusCode }) && proxyRes.headers.location) {
+      // ssg wants to redirect response, e.g. gatsby wants to change page to page/, follow it in proxy server as well
+      const destURL = proxyRes.headers.location
+      res.writeHead(proxyRes.statusCode ?? 200, {
+        Location: destURL,
+        'Cache-Control': 'no-cache',
       })
+      res.end(`Redirecting to ${destURL}`)
+      return
     }
 
     // @ts-expect-error TS(7034) FIXME: Variable 'responseData' implicitly has type 'any[]... Remove this comment to see the full error message
@@ -838,7 +838,7 @@ const onRequest = async (
     hasFormSubmissionHandler &&
     functionsServer &&
     req.method === 'POST' &&
-    !isInternal(req.url) &&
+    !isInternalFunctions(req.url) &&
     (ct.endsWith('/x-www-form-urlencoded') || ct === 'multipart/form-data')
   ) {
     return proxy.web(req, res, { target: functionsServer })

tests/integration/__fixtures__/next-app/app/page.js

@@ -5,6 +5,9 @@ export default function Home() {
   return (
     <main className={styles.main}>
       <div className={styles.description}>
+        <p>
+          Local site Dev Server
+        </p>
         <p>
           Get started by editing&nbsp;
           <code className={styles.code}>app/page.js</code>

tests/integration/__fixtures__/next-app/app/test/exists/page.js

@@ -0,0 +1,7 @@
+export default function ExistsPage() {
+  return (
+    <p>
+      Exists page
+    </p>
+  );
+}

tests/integration/__fixtures__/next-app/netlify.toml

@@ -4,6 +4,16 @@ publish = "public"
 [dev]
 targetPort = 6123
 
+[[redirects]]
+from = "/local/*"
+to = "/:splat"
+status = 200
+
+[[redirects]]
+from = "/test/*"
+to = "https://www.netlify.app/:splat"
+status = 200
+
 [[redirects]]
 from = "*"
 to = "https://www.netlify.app/:splat"

tests/integration/__fixtures__/site-with-redirect/.gitignore

@@ -0,0 +1 @@
+.netlify

tests/integration/__fixtures__/site-with-redirect/index.js

@@ -0,0 +1,25 @@
+const http = require('http')
+
+const server = http.createServer((req, res) => {
+  const pathname = new URL(req.url, 'http://localhost').pathname
+
+  console.log(`Got ${pathname}`)
+
+  if (pathname === '/') {
+    res.write('Root page')
+    res.end()
+  } else if (pathname === '/test/exists') {
+    res.writeHead(302, undefined, { location: '/test/exists/' })
+    res.end()
+  } else if (pathname === '/test/exists/') {
+    res.write('Test exists page')
+    res.end()
+  } else {
+    res.writeHead(404).write('Page is not found')
+    res.end()
+  }
+})
+
+server.listen(6124, () => {
+  console.log('Server is Running')
+})

tests/integration/__fixtures__/site-with-redirect/netlify.toml

@@ -0,0 +1,8 @@
+[dev]
+targetPort = 6124
+command = "npm run dev"
+
+[[redirects]]
+from = "/local/*"
+to = "/:splat"
+status = 200

tests/integration/__fixtures__/site-with-redirect/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "site-with-redirect",
+  "version": "1.0.0",
+  "private": true,
+  "scripts": {
+    "dev": "node index.js"
+  },
+  "dependencies": {
+  }
+}

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

@@ -6,7 +6,7 @@ import fetch from 'node-fetch'
 describe('redirects', () => {
   setupFixtureTests('dev-server-with-functions', { devServer: true }, () => {
     test<FixtureTestContext>('should send original query params to functions', async ({ devServer }) => {
-      const response = await fetch(`http://localhost:${devServer.port}/with-params?param2=world&other=1`, {})
+      const response = await fetch(`http://localhost:${devServer.port}/with-params?param2=world&other=1`)
 
       expect(response.status).toBe(200)
 
@@ -19,7 +19,7 @@ describe('redirects', () => {
     test<FixtureTestContext>('should send original query params to functions when using duplicate parameters', async ({
       devServer,
     }) => {
-      const response = await fetch(`http://localhost:${devServer.port}/api/echo?param=hello&param=world`, {})
+      const response = await fetch(`http://localhost:${devServer.port}/api/echo?param=hello&param=world`)
 
       expect(response.status).toBe(200)
 
@@ -31,23 +31,78 @@ describe('redirects', () => {
 
   setupFixtureTests('next-app', { devServer: { env: { NETLIFY_DEV_SERVER_CHECK_SSG_ENDPOINTS: 1 } } }, () => {
     test<FixtureTestContext>('should prefer local files instead of redirect when not forced', async ({ devServer }) => {
-      const response = await fetch(`http://localhost:${devServer.port}/test.txt`, {})
+      const response = await fetch(`http://localhost:${devServer.port}/test.txt`)
 
       expect(response.status).toBe(200)
 
       const result = await response.text()
       expect(result.trim()).toEqual('hello world')
+      expect(devServer?.output).not.toContain('Proxying to https://www.netlify.app');
     })
 
     test<FixtureTestContext>('should check for the dynamic page existence before doing redirect', async ({
       devServer,
     }) => {
-      const response = await fetch(`http://localhost:${devServer.port}/`, {})
+      const response = await fetch(`http://localhost:${devServer.port}/`)
 
       expect(response.status).toBe(200)
 
       const result = await response.text()
+      expect(result.toLowerCase()).toContain('local site dev server')
       expect(result.toLowerCase()).not.toContain('netlify')
+      expect(devServer?.output).not.toContain('Proxying to https://www.netlify.app');
+    })
+
+    test<FixtureTestContext>('nested route redirect check for the page existence', async ({
+      devServer,
+    }) => {
+      let response = await fetch(`http://localhost:${devServer.port}/test/exists`)
+      expect(response.status).toBe(200)
+
+      let result = await response.text()
+      expect(result.toLowerCase()).toContain('exists page')
+      expect(devServer?.output).not.toContain('Proxying to https://www.netlify.app/exists');
+
+      response = await fetch(`http://localhost:${devServer.port}/test/about`)
+      expect(response.status).toBe(200)
+
+      result = await response.text()
+      expect(result.toLowerCase()).toContain('netlify')
+
+      expect(devServer?.output).toContain('Proxying to https://www.netlify.app/about');
+    })
+
+    test<FixtureTestContext>('should do local redirect', async ({
+      devServer,
+    }) => {
+      const response = await fetch(`http://localhost:${devServer.port}/local/test/exists`)
+
+      expect(response.status).toBe(200)
+
+      const result = await response.text()
+      expect(response.headers.get('location')).toBeNull()
+      expect(response.status).toBe(200)
+      expect(result.toLowerCase()).toContain('exists page')
+      expect(result.toLowerCase()).not.toContain('netlify')
+      expect(devServer?.output).not.toContain('Proxying to https://www.netlify.app/test');
+    })
+  })
+
+  setupFixtureTests('site-with-redirect', { devServer: true }, () => {
+    test<FixtureTestContext>('should do local redirect', async ({
+      devServer,
+    }) => {
+      const response = await fetch(`http://localhost:${devServer.port}/local/test/exists`)
+
+      expect(response.status).toBe(200)
+
+      const result = await response.text()
+      expect(response.headers.get('location')).toBeNull()
+      expect(response.url).toEqual(`http://localhost:${devServer.port}/test/exists/`)
+      expect(response.status).toBe(200)
+      expect(result.toLowerCase()).toContain('exists page')
+      expect(result.toLowerCase()).not.toContain('netlify')
+      expect(devServer?.output).not.toContain('Proxying to https://www.netlify.app');
     })
   })
 })