fix streaming on edge for bots

vercel · Mar 3, 2022 · 3eda14e · 3eda14e
1 parent b325b1b
commit 3eda14e
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 93 deletions.
diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -980,20 +980,15 @@ export default abstract class Server {
       query: NextParsedUrlQuery
     }
   ): Promise<void> {
-    const userAgent = partialContext.req.headers['user-agent']
+    const isBotRequest = isBot(partialContext.req.headers['user-agent'] || '')
     const ctx = {
       ...partialContext,
       renderOpts: {
         ...this.renderOpts,
-        supportsDynamicHTML: userAgent
-          ? !isBot(userAgent)
-          : !!this.renderOpts.supportsDynamicHTML,
+        supportsDynamicHTML: !isBotRequest,
       },
     } as const
-    let payload: ResponsePayload | null = null
-    try {
-      payload = await fn(ctx)
-    } catch (_) {}
+    const payload = await fn(ctx)
     if (payload === null) {
       return
     }
@@ -1180,11 +1175,13 @@ export default abstract class Server {
     }
 
     if (opts.supportsDynamicHTML === true) {
+      const isBotRequest = isBot(req.headers['user-agent'] || '')
       // Disable dynamic HTML in cases that we know it won't be generated,
       // so that we can continue generating a cache key when possible.
       opts.supportsDynamicHTML =
         !isSSG &&
         !isLikeServerless &&
+        !isBotRequest &&
         !query.amp &&
         typeof components.Document?.getInitialProps !== 'function'
     }

diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx
@@ -1745,7 +1745,7 @@ function createFlushEffectStream(
   })
 }
 
-function renderToStream({
+async function renderToStream({
   ReactDOMServer,
   element,
   suffix,
@@ -1760,73 +1760,53 @@ function renderToStream({
   generateStaticHTML: boolean
   flushEffectHandler?: () => Promise<string>
 }): Promise<ReadableStream<Uint8Array>> {
-  return new Promise(async (resolve, reject) => {
-    const closeTag = '</body></html>'
-    const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null
-
-    const doResolve = execOnce((renderStream: ReadableStream<Uint8Array>) => {
-      // React will call our callbacks synchronously, so we need to
-      // defer to a microtask to ensure `stream` is set.
-      resolve(
-        Promise.resolve().then(() => {
-          const transforms: Array<TransformStream<Uint8Array, Uint8Array>> = [
-            createBufferedTransformStream(),
-            flushEffectHandler
-              ? createFlushEffectStream(flushEffectHandler)
-              : null,
-            suffixUnclosed != null ? createPrefixStream(suffixUnclosed) : null,
-            dataStream ? createInlineDataStream(dataStream) : null,
-            suffixUnclosed != null ? createSuffixStream(closeTag) : null,
-          ].filter(Boolean) as any
-
-          return transforms.reduce(
-            (readable, transform) => pipeThrough(readable, transform),
-            renderStream
-          )
-        })
-      )
-    })
-
-    let completeCallback: (value?: unknown) => void
-    const allComplete = new Promise((resolveError, rejectError) => {
-      completeCallback = execOnce((err: unknown) => {
-        if (err) {
-          rejectError(err)
-        } else {
-          resolveError(null)
-        }
-      })
-    })
+  const closeTag = '</body></html>'
+  const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null
+
+  const doResolve = (renderStream: ReadableStream<Uint8Array>) => {
+    // React will call our callbacks synchronously, so we need to
+    // defer to a microtask to ensure `stream` is set.
+    const transforms: Array<TransformStream<Uint8Array, Uint8Array>> = [
+      createBufferedTransformStream(),
+      flushEffectHandler ? createFlushEffectStream(flushEffectHandler) : null,
+      suffixUnclosed != null ? createPrefixStream(suffixUnclosed) : null,
+      dataStream ? createInlineDataStream(dataStream) : null,
+      suffixUnclosed != null ? createSuffixStream(closeTag) : null,
+    ].filter(Boolean) as any
+
+    return transforms.reduce(
+      (readable, transform) => pipeThrough(readable, transform),
+      renderStream
+    )
+  }
 
-    async function bailOnError() {
-      try {
-        await allComplete
-      } catch (err) {
-        reject(err)
+  let completeCallback: (value?: unknown) => void
+  const allComplete = new Promise((resolveError, rejectError) => {
+    completeCallback = execOnce((err: unknown) => {
+      if (err) {
+        rejectError(err)
+      } else {
+        resolveError(null)
       }
-    }
-
-    const renderStream: ReadableStream<Uint8Array> = await (
-      ReactDOMServer as any
-    ).renderToReadableStream(element, {
-      onError(err: Error) {
-        if (generateStaticHTML) {
-          completeCallback(err)
-        } else {
-          reject(err)
-        }
-      },
-      onCompleteAll() {
-        completeCallback()
-      },
     })
+  })
 
-    if (generateStaticHTML) {
-      await bailOnError()
-    }
-
-    doResolve(renderStream)
+  const renderStream: ReadableStream<Uint8Array> = await (
+    ReactDOMServer as any
+  ).renderToReadableStream(element, {
+    onError(err: Error) {
+      completeCallback(err)
+    },
+    onCompleteAll() {
+      completeCallback()
+    },
   })
+
+  if (generateStaticHTML) {
+    await allComplete
+  }
+
+  return doResolve(renderStream)
 }
 
 function encodeText(input: string) {

diff --git a/packages/next/server/web-server.ts b/packages/next/server/web-server.ts
@@ -3,9 +3,9 @@ import type { RenderOpts } from './render'
 import type RenderResult from './render-result'
 import type { NextParsedUrlQuery } from './request-meta'
 import type { Params } from './router'
+import type { PayloadOptions } from './send-payload'
 import type { LoadComponentsReturnType } from './load-components'
 
-import { PayloadOptions } from './send-payload'
 import BaseServer, { Options } from './base-server'
 import { renderToHTML } from './render'
 
@@ -131,6 +131,7 @@ export default class NextWebServer extends BaseServer {
       query,
       {
         ...renderOpts,
+        // supportsDynamicHTML: true,
         disableOptimizedLoading: true,
         runtime: 'edge',
       }

diff --git a/test/integration/react-streaming-and-server-components/test/basic.js b/test/integration/react-streaming-and-server-components/test/basic.js
@@ -37,26 +37,12 @@ export default async function basic(context, { env }) {
   })
 
   it('should render 500 error correctly', async () => {
-    const errGipHTML = await renderViaHTTP(context.appPort, '/err')
-    const errSuspenseHTML = await renderViaHTTP(
-      context.appPort,
-      '/err/suspense'
-    )
-    const errSuspenseRender = await renderViaHTTP(
-      context.appPort,
-      '/err/render'
-    )
-
-    ;[errGipHTML, errSuspenseHTML, errSuspenseRender].forEach((html, index) => {
-      if (env === 'dev') {
-        // TODO: extract suspense error in streaming properly
-        if (index === 0) {
-          // In dev mode it should show the error popup.
-          expect(html).toContain('Error: oops')
-        }
-      } else {
-        expect(html).toContain('custom-500-page')
-      }
-    })
+    const html = await renderViaHTTP(context.appPort, '/err')
+    if (env === 'dev') {
+      // In dev mode it should show the error popup.
+      expect(html).toContain('Error: oops')
+    } else {
+      expect(html).toContain('custom-500-page')
+    }
   })
 }