Include server latency in navigation debug info

The debug info of the promise created by the Flight client represents
the latency between when the navigation starts and when it's passed
to React. Most importantly, it should include the time it takes for the
client to start receiving data from the server.

Before this PR, the timing was too late because we did not call into
the Flight client until after we started receiving data.

To fix this, we switch from using `createFromReadableStream` to
`createFromFetch`. This allows us to call into the Flight client
immediately without waiting for the `fetch` promise to resolve.

Author: acdlite

packages/next/src/client/components/router-reducer/fetch-server-response.ts

@@ -2,7 +2,10 @@
 
 // TODO: Explicitly import from client.browser
 // eslint-disable-next-line import/no-extraneous-dependencies
-import { createFromReadableStream as createFromReadableStreamBrowser } from 'react-server-dom-webpack/client'
+import {
+  createFromReadableStream as createFromReadableStreamBrowser,
+  createFromFetch as createFromFetchBrowser,
+} from 'react-server-dom-webpack/client'
 
 import type {
   FlightRouterState,
@@ -36,6 +39,8 @@ import { urlToUrlWithoutFlightMarker } from '../../route-params'
 
 const createFromReadableStream =
   createFromReadableStreamBrowser as (typeof import('react-server-dom-webpack/client.browser'))['createFromReadableStream']
+const createFromFetch =
+  createFromFetchBrowser as (typeof import('react-server-dom-webpack/client.browser'))['createFromFetch']
 
 let createDebugChannel:
   | typeof import('../../dev/debug-channel').createDebugChannel
@@ -170,10 +175,17 @@ export async function fetchServerResponse(
       }
     }
 
-    const res = await createFetch(
+    // Typically, during a navigation, we decode the response using Flight's
+    // `createFromFetch` API, which accepts a `fetch` promise.
+    // TODO: Remove this check once the old PPR flag is removed
+    const isLegacyPPR =
+      process.env.__NEXT_PPR && !process.env.__NEXT_CACHE_COMPONENTS
+    const shouldImmediatelyDecode = !isLegacyPPR
+    const res = await createFetch<NavigationFlightResponse>(
       url,
       headers,
       fetchPriority,
+      shouldImmediatelyDecode,
       abortController.signal
     )
 
@@ -221,24 +233,34 @@ export async function fetchServerResponse(
       ).waitForWebpackRuntimeHotUpdate()
     }
 
-    // Handle the `fetch` readable stream that can be unwrapped by `React.use`.
-    const flightStream = postponed
-      ? createUnclosingPrefetchStream(res.body)
-      : res.body
-    const response = await (createFromNextReadableStream(
-      flightStream,
-      res.headers
-    ) as Promise<NavigationFlightResponse>)
+    let flightResponsePromise = res.flightResponse
+    if (flightResponsePromise === null) {
+      // Typically, `createFetch` would have already started decoding the
+      // Flight response. If it hasn't, though, we need to decode it now.
+      // TODO: This should only be reachable if legacy PPR is enabled (i.e. PPR
+      // without Cache Components). Remove this branch once legacy PPR
+      // is deleted.
+      const flightStream = postponed
+        ? createUnclosingPrefetchStream(res.body)
+        : res.body
+      flightResponsePromise =
+        createFromNextReadableStream<NavigationFlightResponse>(
+          flightStream,
+          res.headers
+        )
+    }
+
+    const flightResponse = await flightResponsePromise
 
-    if (getAppBuildId() !== response.b) {
+    if (getAppBuildId() !== flightResponse.b) {
       return doMpaNavigation(res.url)
     }
 
     return {
-      flightData: normalizeFlightData(response.f),
+      flightData: normalizeFlightData(flightResponse.f),
       canonicalUrl: canonicalUrl,
       couldBeIntercepted: interception,
-      prerendered: response.S,
+      prerendered: flightResponse.S,
       postponed,
       staleTime,
     }
@@ -269,21 +291,23 @@ export async function fetchServerResponse(
 // the codebase. For example, there's some custom logic for manually following
 // redirects, so "redirected" in this type could be a composite of multiple
 // browser fetch calls; however, this fact should not leak to the caller.
-export type RSCResponse = {
+export type RSCResponse<T> = {
   ok: boolean
   redirected: boolean
   headers: Headers
   body: ReadableStream<Uint8Array> | null
   status: number
   url: string
+  flightResponse: (Promise<T> & { _debugInfo?: Array<any> }) | null
 }
 
-export async function createFetch(
+export async function createFetch<T>(
   url: URL,
   headers: RequestHeaders,
   fetchPriority: 'auto' | 'high' | 'low' | null,
+  shouldImmediatelyDecode: boolean,
   signal?: AbortSignal
-): Promise<RSCResponse> {
+): Promise<RSCResponse<T>> {
   // TODO: In output: "export" mode, the headers do nothing. Omit them (and the
   // cache busting search param) from the request so they're
   // maximally cacheable.
@@ -312,7 +336,21 @@ export async function createFetch(
   // track them separately.
   let fetchUrl = new URL(url)
   setCacheBustingSearchParam(fetchUrl, headers)
-  let browserResponse = await fetch(fetchUrl, fetchOptions)
+  let fetchPromise = fetch(fetchUrl, fetchOptions)
+  // Immediately pass the fetch promise to the Flight client so that the debug
+  // info includes the latency from the client to the server. The internal timer
+  // in React starts as soon as `createFromFetch` is called.
+  //
+  // The only case where we don't do this is during a prefetch, because we have
+  // to do some extra processing of the response stream (see
+  // `createUnclosingPrefetchStream`). But this is fine, because a top-level
+  // prefetch response never blocks a navigation; if it hasn't already been
+  // written into the cache by the time the navigation happens, the router will
+  // go straight to a dynamic request.
+  let flightResponsePromise = shouldImmediatelyDecode
+    ? createFromNextFetch<T>(fetchPromise)
+    : null
+  let browserResponse = await fetchPromise
 
   // If the server responds with a redirect (e.g. 307), and the redirected
   // location does not contain the cache busting search param set in the
@@ -365,9 +403,14 @@ export async function createFetch(
       //
       // Append the cache busting search param to the redirected URL and
       // fetch again.
+      // TODO: We should abort the previous request.
       fetchUrl = new URL(responseUrl)
       setCacheBustingSearchParam(fetchUrl, headers)
-      browserResponse = await fetch(fetchUrl, fetchOptions)
+      fetchPromise = fetch(fetchUrl, fetchOptions)
+      flightResponsePromise = shouldImmediatelyDecode
+        ? createFromNextFetch<T>(fetchPromise)
+        : null
+      browserResponse = await fetchPromise
       // We just performed a manual redirect, so this is now true.
       redirected = true
     }
@@ -378,7 +421,7 @@ export async function createFetch(
   const responseUrl = new URL(browserResponse.url, fetchUrl)
   responseUrl.searchParams.delete(NEXT_RSC_UNION_QUERY)
 
-  const rscResponse: RSCResponse = {
+  const rscResponse: RSCResponse<T> = {
     url: responseUrl.href,
 
     // This is true if any redirects occurred, either automatically by the
@@ -394,22 +437,36 @@ export async function createFetch(
     headers: browserResponse.headers,
     body: browserResponse.body,
     status: browserResponse.status,
+
+    // This is the exact promise returned by `createFromFetch`. It contains
+    // debug information that we need to transfer to any derived promises that
+    // are later rendered by React.
+    flightResponse: flightResponsePromise,
   }
 
   return rscResponse
 }
 
-export function createFromNextReadableStream(
+export function createFromNextReadableStream<T>(
   flightStream: ReadableStream<Uint8Array>,
   responseHeaders: Headers
-): Promise<unknown> {
+): Promise<T> & { _debugInfo?: Array<any> } {
   return createFromReadableStream(flightStream, {
     callServer,
     findSourceMapURL,
     debugChannel: createDebugChannel && createDebugChannel(responseHeaders),
   })
 }
 
+function createFromNextFetch<T>(
+  promiseForResponse: Promise<Response>
+): Promise<T> & { _debugInfo?: Array<any> } {
+  return createFromFetch(promiseForResponse, {
+    callServer,
+    findSourceMapURL,
+  })
+}
+
 function createUnclosingPrefetchStream(
   originalFlightStream: ReadableStream<Uint8Array>
 ): ReadableStream<Uint8Array> {

packages/next/src/client/components/segment-cache-impl/cache.ts

@@ -1430,10 +1430,10 @@ export async function fetchRouteOnCacheMiss(
           routeCacheLru.updateSize(entry, size)
         }
       )
-      const serverData = await (createFromNextReadableStream(
+      const serverData = await createFromNextReadableStream<RootTreePrefetch>(
         prefetchStream,
         response.headers
-      ) as Promise<RootTreePrefetch>)
+      )
       if (serverData.buildId !== getAppBuildId()) {
         // The server build does not match the client. Treat as a 404. During
         // an actual navigation, the router will trigger an MPA navigation.
@@ -1482,10 +1482,11 @@ export async function fetchRouteOnCacheMiss(
           routeCacheLru.updateSize(entry, size)
         }
       )
-      const serverData = await (createFromNextReadableStream(
-        prefetchStream,
-        response.headers
-      ) as Promise<NavigationFlightResponse>)
+      const serverData =
+        await createFromNextReadableStream<NavigationFlightResponse>(
+          prefetchStream,
+          response.headers
+        )
       if (serverData.b !== getAppBuildId()) {
         // The server build does not match the client. Treat as a 404. During
         // an actual navigation, the router will trigger an MPA navigation.
@@ -1503,7 +1504,7 @@ export async function fetchRouteOnCacheMiss(
         // The non-PPR response format is what we'd get if we prefetched these segments
         // using the LoadingBoundary fetch strategy, so mark their cache entries accordingly.
         FetchStrategy.LoadingBoundary,
-        response,
+        response as RSCResponse<NavigationFlightResponse>,
         serverData,
         entry,
         couldBeIntercepted,
@@ -1768,7 +1769,7 @@ export async function fetchSegmentPrefetchesUsingDynamicRequest(
       Date.now(),
       task,
       fetchStrategy,
-      response,
+      response as RSCResponse<NavigationFlightResponse>,
       serverData,
       isResponsePartial,
       route,
@@ -1791,7 +1792,7 @@ function writeDynamicTreeResponseIntoCache(
     | FetchStrategy.LoadingBoundary
     | FetchStrategy.PPRRuntime
     | FetchStrategy.Full,
-  response: RSCResponse,
+  response: RSCResponse<NavigationFlightResponse>,
   serverData: NavigationFlightResponse,
   entry: PendingRouteCacheEntry,
   couldBeIntercepted: boolean,
@@ -1896,7 +1897,7 @@ function writeDynamicRenderResponseIntoCache(
     | FetchStrategy.LoadingBoundary
     | FetchStrategy.PPRRuntime
     | FetchStrategy.Full,
-  response: RSCResponse,
+  response: RSCResponse<NavigationFlightResponse>,
   serverData: NavigationFlightResponse,
   isResponsePartial: boolean,
   route: FulfilledRouteCacheEntry,
@@ -2120,12 +2121,22 @@ function writeSeedDataIntoCache(
   }
 }
 
-async function fetchPrefetchResponse(
+async function fetchPrefetchResponse<T>(
   url: URL,
   headers: RequestHeaders
-): Promise<RSCResponse | null> {
+): Promise<RSCResponse<T> | null> {
   const fetchPriority = 'low'
-  const response = await createFetch(url, headers, fetchPriority)
+  // When issuing a prefetch request, don't immediately decode the response; we
+  // use the lower level `createFromResponse` API instead because we need to do
+  // some extra processing of the response stream. See
+  // `createPrefetchResponseStream` for more details.
+  const shouldImmediatelyDecode = false
+  const response = await createFetch<T>(
+    url,
+    headers,
+    fetchPriority,
+    shouldImmediatelyDecode
+  )
   if (!response.ok) {
     return null
   }

packages/next/src/client/route-params.ts

@@ -21,7 +21,9 @@ export type RouteParam = {
   type: DynamicParamTypesShort
 }
 
-export function getRenderedSearch(response: RSCResponse): NormalizedSearch {
+export function getRenderedSearch(
+  response: RSCResponse<unknown>
+): NormalizedSearch {
   // If the server performed a rewrite, the search params used to render the
   // page will be different from the params in the request URL. In this case,
   // the response will include a header that gives the rewritten search query.
@@ -37,7 +39,7 @@ export function getRenderedSearch(response: RSCResponse): NormalizedSearch {
     .search as NormalizedSearch
 }
 
-export function getRenderedPathname(response: RSCResponse): string {
+export function getRenderedPathname(response: RSCResponse<unknown>): string {
   // If the server performed a rewrite, the pathname used to render the
   // page will be different from the pathname in the request URL. In this case,
   // the response will include a header that gives the rewritten pathname.