[turbopack]: Fix HEAD requests (#50366)

I noticed while testing that we're getting a bunch of 500 errors after #50241 merged. Turns out that `fetchNextData` will [fetch `HEAD` requests](https://github.com/vercel/next.js/blob/cf9591cd/packages/next/src/shared/lib/router/router.ts#L619-L621) for background priority. The problem is that, somewhere, the Next router is draining the body from HEAD responses, leading us to trying to `JSON.parse` an empty string.

This changes the way we return results to the Turbopack router. Instead of `JSON.stringify`ing the result into the body (which will be drained by something), we directly return the result. And it saves us a `stringify` -> `parse` -> `stringify` round trip, so that's nice.

I took the chance to clean up some of our boilerplate code, too.

packages/next-swc/crates/next-core/js/src/entry/router.ts

@@ -1,12 +1,9 @@
 import type { Ipc, StructuredError } from '@vercel/turbopack-node/ipc/index'
-import type { IncomingMessage, ServerResponse } from 'node:http'
+import type { IncomingMessage } from 'node:http'
 import { Buffer } from 'node:buffer'
 import { createServer, makeRequest, type ServerInfo } from '../internal/server'
 import { toPairs } from '../internal/headers'
-import {
-  makeResolver,
-  type RouteResult,
-} from 'next/dist/server/lib/route-resolver'
+import { makeResolver } from 'next/dist/server/lib/route-resolver'
 import loadConfig from 'next/dist/server/config'
 import { PHASE_DEVELOPMENT_SERVER } from 'next/dist/shared/lib/constants'
 
@@ -110,71 +107,61 @@ export default async function route(
     // the serverRequest to Next.js to handle.
     clientRequest.end(body)
 
-    // The route promise must not block us from starting the client response
-    // handling, so we cannot await it yet. By making the call, we allow
-    // Next.js to start writing to the response whenever it's ready.
+    // The route promise must not block us from starting the middleware
+    // response handling, so we cannot await it yet. By making the call, we
+    // allow Next.js to start writing to the response whenever it's ready.
     const routePromise = resolveRoute(serverRequest, serverResponse)
 
-    // Now that the Next.js has started processing the route, the
-    // clientResponsePromise will resolve once they write data and then we can
-    // begin streaming.
-    // We again cannot block on the clientResponsePromise, because an error may
+    // Now that the Next.js has started processing the route, the middleware
+    // response promise will resolve once they write data and then we can begin
+    // streaming.
+    // We again cannot await directly on the promise, because an error may
     // occur in the routePromise while we're waiting.
-    const responsePromise = clientResponsePromise.then((c) =>
-      handleClientResponse(ipc, c)
+    const middlewarePromise = clientResponsePromise.then((c) =>
+      handleMiddlewareResponse(ipc, c)
     )
 
     // Now that both promises are in progress, we await both so that a
     // rejection in either will end the routing.
-    const [response] = await Promise.all([responsePromise, routePromise])
+    const [routeResult] = await Promise.all([routePromise, middlewarePromise])
 
     server.close()
-    return response
+
+    if (routeResult) {
+      switch (routeResult.type) {
+        case 'none':
+        case 'error':
+          return routeResult
+        case 'rewrite':
+          return {
+            type: 'rewrite',
+            data: {
+              url: routeResult.url,
+              headers: Object.entries(routeResult.headers)
+                .filter(([, val]) => val != null)
+                .map(([name, value]) => [name, value!.toString()]),
+            },
+          }
+        default:
+          // @ts-expect-error data.type is never
+          throw new Error(`unknown route result type: ${data.type}`)
+      }
+    }
   } catch (e) {
     // Server doesn't need to be closed, because the sendError will terminate
     // the process.
     ipc.sendError(e as Error)
   }
 }
 
-async function handleClientResponse(
+async function handleMiddlewareResponse(
   ipc: Ipc<RouterRequest, IpcOutgoingMessage>,
   clientResponse: IncomingMessage
-): Promise<MessageData | void> {
-  if (clientResponse.headers['x-nextjs-route-result'] === '1') {
-    clientResponse.setEncoding('utf8')
-    // We're either a redirect or a rewrite
-    let buffer = ''
-    for await (const chunk of clientResponse) {
-      buffer += chunk
-    }
-
-    const data = JSON.parse(buffer) as RouteResult
-
-    switch (data.type) {
-      case 'none':
-        return {
-          type: 'none',
-        }
-      case 'error':
-        return {
-          type: 'error',
-          error: data.error,
-        }
-      case 'rewrite':
-        return {
-          type: 'rewrite',
-          data: {
-            url: data.url,
-            headers: Object.entries(data.headers)
-              .filter(([, val]) => val != null)
-              .map(([name, value]) => [name, value!.toString()]),
-          },
-        }
-      default:
-        // @ts-expect-error data.type is never
-        throw new Error(`unknown route result type: ${data.type}`)
-    }
+): Promise<void> {
+  // If this header is specified, we know that the response was not handled by
+  // middleware. The headers and body of the response are useless.
+  if (clientResponse.headers['x-nextjs-route-result']) {
+    return
   }
 
   const responseHeaders: MiddlewareHeadersResponse = {

packages/next-swc/crates/next-core/js/src/internal/server.ts

@@ -40,7 +40,6 @@ export function makeRequest(
   serverResponse: ServerResponse<IncomingMessage>
 }> {
   return new Promise((resolve, reject) => {
-    let clientRequest: ClientRequest | null = null
     let clientResponseResolve: (value: IncomingMessage) => void
     let clientResponseReject: (error: Error) => void
     const clientResponsePromise = new Promise<IncomingMessage>(
@@ -49,59 +48,23 @@ export function makeRequest(
         clientResponseReject = reject
       }
     )
-    let serverRequest: IncomingMessage | null = null
-    let serverResponse: ServerResponse<IncomingMessage> | null = null
-
-    const maybeResolve = () => {
-      if (
-        clientRequest != null &&
-        serverRequest != null &&
-        serverResponse != null
-      ) {
-        cleanup()
-        resolve({
-          clientRequest,
-          clientResponsePromise,
-          serverRequest,
-          serverResponse,
-        })
-      }
-    }
-
-    const cleanup = () => {
-      server.removeListener('error', errorListener)
-      server.removeListener('request', requestListener)
-    }
 
     const errorListener = (err: Error) => {
-      cleanup()
+      server.removeListener('request', requestListener)
       reject(err)
     }
 
     const requestListener = (
       req: IncomingMessage,
       res: ServerResponse<IncomingMessage>
     ) => {
-      serverRequest = req
-      serverResponse = res
-      maybeResolve()
-    }
-
-    const cleanupClientResponse = () => {
-      if (clientRequest != null) {
-        clientRequest.removeListener('response', responseListener)
-        clientRequest.removeListener('error', clientResponseErrorListener)
-      }
-    }
-
-    const clientResponseErrorListener = (err: Error) => {
-      cleanupClientResponse()
-      clientResponseReject(err)
-    }
-
-    const responseListener = (res: IncomingMessage) => {
-      cleanupClientResponse()
-      clientResponseResolve(res)
+      server.removeListener('error', errorListener)
+      resolve({
+        clientRequest,
+        clientResponsePromise,
+        serverRequest: req,
+        serverResponse: res,
+      })
     }
 
     server.once('request', requestListener)
@@ -112,18 +75,27 @@ export function makeRequest(
     const headers = headersFromEntries(rawHeaders ?? [])
     initProxiedHeaders(headers, proxiedFor)
 
-    clientRequest = http.request({
+    const clientRequest = http.request({
       host: 'localhost',
       port: address.port,
       method,
-      path:
-        rawQuery != null && rawQuery.length > 0 ? `${path}?${rawQuery}` : path,
+      path: rawQuery?.length ? `${path}?${rawQuery}` : path,
       headers,
     })
 
     // Otherwise Node.js waits for the first chunk of data to be written before sending the request.
     clientRequest.flushHeaders()
 
+    const clientResponseErrorListener = (err: Error) => {
+      clientRequest.removeListener('response', responseListener)
+      clientResponseReject(err)
+    }
+
+    const responseListener = (res: IncomingMessage) => {
+      clientRequest.removeListener('error', clientResponseErrorListener)
+      clientResponseResolve(res)
+    }
+
     clientRequest.once('response', responseListener)
     clientRequest.once('error', clientResponseErrorListener)
   })

packages/next/src/server/lib/route-resolver.ts

@@ -240,7 +240,7 @@ export async function makeResolver(
   return async function resolveRoute(
     _req: IncomingMessage,
     _res: ServerResponse
-  ) {
+  ): Promise<RouteResult | void> {
     const req = new NodeNextRequest(_req)
     const res = new NodeNextResponse(_res)
     const parsedUrl = url.parse(req.url!, true)
@@ -250,15 +250,28 @@ export async function makeResolver(
 
     await router.execute(req, res, parsedUrl)
 
-    if (!res.originalResponse.headersSent) {
-      res.setHeader('x-nextjs-route-result', '1')
-      const routeResult: RouteResult = routeResults.get(req) ?? {
-        type: 'none',
-      }
+    // If the headers are sent, then this was handled by middleware and there's
+    // nothing for us to do.
+    if (res.originalResponse.headersSent) {
+      return
+    }
 
-      res.body(JSON.stringify(routeResult)).send()
+    // The response won't be used, but we need to close the request so that the
+    // ClientResponse's promise will resolve. We signal that this response is
+    // unneeded via the header.
+    res.setHeader('x-nextjs-route-result', '1')
+    res.send()
+
+    // If we have a routeResult, then we hit the catchAllRoute during execution
+    // and this is a rewrite request.
+    const routeResult = routeResults.get(req)
+    if (routeResult) {
+      routeResults.delete(req)
+      return routeResult
     }
 
-    routeResults.delete(req)
+    // Finally, if the catchall didn't match, than this request is invalid
+    // (maybe they're missing the basePath?)
+    return { type: 'none' }
   }
 }