From 47ead3b33a7f19c846a739fc47d542a8532cb257 Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Tue, 10 Mar 2020 15:28:13 -0400
Subject: [PATCH] refactor(ssr): improve ssr async setup / suspense error
 handling

---
 packages/runtime-core/src/component.ts        |  4 +-
 .../__tests__/ssrSuspense.spec.ts             | 18 +++++-
 .../server-renderer/src/renderToString.ts     | 56 +++++++++++++------
 3 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts
index ad8453e9c..05112c70f 100644
--- a/packages/runtime-core/src/component.ts
+++ b/packages/runtime-core/src/component.ts
@@ -337,7 +337,7 @@ function setupStatefulComponent(
   // 2. create props proxy
   // the propsProxy is a reactive AND readonly proxy to the actual props.
   // it will be updated in resolveProps() on updates before render
-  const propsProxy = (instance.propsProxy = isInSSRComponentSetup
+  const propsProxy = (instance.propsProxy = isSSR
     ? instance.props
     : shallowReadonly(instance.props))
   // 3. call setup()
@@ -360,7 +360,7 @@ function setupStatefulComponent(
     currentSuspense = null
 
     if (isPromise(setupResult)) {
-      if (isInSSRComponentSetup) {
+      if (isSSR) {
         // return the promise so server-renderer can wait on it
         return setupResult.then(resolvedResult => {
           handleSetupResult(instance, resolvedResult, parentSuspense, isSSR)
diff --git a/packages/server-renderer/__tests__/ssrSuspense.spec.ts b/packages/server-renderer/__tests__/ssrSuspense.spec.ts
index b411e14a7..c4c3b2d5a 100644
--- a/packages/server-renderer/__tests__/ssrSuspense.spec.ts
+++ b/packages/server-renderer/__tests__/ssrSuspense.spec.ts
@@ -2,6 +2,16 @@ import { createApp, h, Suspense } from 'vue'
 import { renderToString } from '../src/renderToString'
 
 describe('SSR Suspense', () => {
+  let logError: jest.SpyInstance
+
+  beforeEach(() => {
+    logError = jest.spyOn(console, 'error').mockImplementation(() => {})
+  })
+
+  afterEach(() => {
+    logError.mockRestore()
+  })
+
   const ResolvingAsync = {
     async setup() {
       return () => h('div', 'async')
@@ -10,7 +20,7 @@ describe('SSR Suspense', () => {
 
   const RejectingAsync = {
     setup() {
-      return new Promise((_, reject) => reject())
+      return new Promise((_, reject) => reject('foo'))
     }
   }
 
@@ -25,6 +35,7 @@ describe('SSR Suspense', () => {
     }
 
     expect(await renderToString(createApp(Comp))).toBe(`<div>async</div>`)
+    expect(logError).not.toHaveBeenCalled()
   })
 
   test('fallback', async () => {
@@ -38,6 +49,7 @@ describe('SSR Suspense', () => {
     }
 
     expect(await renderToString(createApp(Comp))).toBe(`<div>fallback</div>`)
+    expect(logError).toHaveBeenCalled()
   })
 
   test('2 components', async () => {
@@ -53,6 +65,7 @@ describe('SSR Suspense', () => {
     expect(await renderToString(createApp(Comp))).toBe(
       `<div><div>async</div><div>async</div></div>`
     )
+    expect(logError).not.toHaveBeenCalled()
   })
 
   test('resolving component + rejecting component', async () => {
@@ -66,6 +79,7 @@ describe('SSR Suspense', () => {
     }
 
     expect(await renderToString(createApp(Comp))).toBe(`<div>fallback</div>`)
+    expect(logError).toHaveBeenCalled()
   })
 
   test('failing suspense in passing suspense', async () => {
@@ -87,6 +101,7 @@ describe('SSR Suspense', () => {
     expect(await renderToString(createApp(Comp))).toBe(
       `<div><div>async</div><div>fallback 2</div></div>`
     )
+    expect(logError).toHaveBeenCalled()
   })
 
   test('passing suspense in failing suspense', async () => {
@@ -106,5 +121,6 @@ describe('SSR Suspense', () => {
     }
 
     expect(await renderToString(createApp(Comp))).toBe(`<div>fallback 1</div>`)
+    expect(logError).toHaveBeenCalled()
   })
 })
diff --git a/packages/server-renderer/src/renderToString.ts b/packages/server-renderer/src/renderToString.ts
index 8051c204b..ee9dd8143 100644
--- a/packages/server-renderer/src/renderToString.ts
+++ b/packages/server-renderer/src/renderToString.ts
@@ -10,7 +10,6 @@ import {
   Fragment,
   ssrUtils,
   Slots,
-  warn,
   createApp,
   ssrContextKey
 } from 'vue'
@@ -139,6 +138,8 @@ export function renderComponent(
   )
 }
 
+export const AsyncSetupErrorMarker = Symbol('Vue async setup error')
+
 function renderComponentVNode(
   vnode: VNode,
   parentComponent: ComponentInternalInstance | null = null
@@ -150,7 +151,21 @@ function renderComponentVNode(
     true /* isSSR */
   )
   if (isPromise(res)) {
-    return res.then(() => renderComponentSubTree(instance))
+    return res
+      .catch(err => {
+        // normalize async setup rejection
+        if (!(err instanceof Error)) {
+          err = new Error(String(err))
+        }
+        err[AsyncSetupErrorMarker] = true
+        console.error(
+          `[@vue/server-renderer]: Uncaught error in async setup:\n`,
+          err
+        )
+        // rethrow for suspense
+        throw err
+      })
+      .then(() => renderComponentSubTree(instance))
   } else {
     return renderComponentSubTree(instance)
   }
@@ -208,7 +223,9 @@ function ssrCompile(
     isNativeTag: instance.appContext.config.isNativeTag || NO,
     onError(err: CompilerError) {
       if (__DEV__) {
-        const message = `Template compilation error: ${err.message}`
+        const message = `[@vue/server-renderer] Template compilation error: ${
+          err.message
+        }`
         const codeFrame =
           err.loc &&
           generateCodeFrame(
@@ -216,7 +233,7 @@ function ssrCompile(
             err.loc.start.offset,
             err.loc.end.offset
           )
-        warn(codeFrame ? `${message}\n${codeFrame}` : message)
+        console.error(codeFrame ? `${message}\n${codeFrame}` : message)
       } else {
         throw err
       }
@@ -243,15 +260,15 @@ function renderVNode(
       break
     default:
       if (shapeFlag & ShapeFlags.ELEMENT) {
-        renderElement(push, vnode, parentComponent)
+        renderElementVNode(push, vnode, parentComponent)
       } else if (shapeFlag & ShapeFlags.COMPONENT) {
         push(renderComponentVNode(vnode, parentComponent))
       } else if (shapeFlag & ShapeFlags.PORTAL) {
-        renderPortal(vnode, parentComponent)
+        renderPortalVNode(vnode, parentComponent)
       } else if (shapeFlag & ShapeFlags.SUSPENSE) {
-        push(renderSuspense(vnode, parentComponent))
+        push(renderSuspenseVNode(vnode, parentComponent))
       } else {
-        console.warn(
+        console.error(
           '[@vue/server-renderer] Invalid VNode type:',
           type,
           `(${typeof type})`
@@ -270,7 +287,7 @@ export function renderVNodeChildren(
   }
 }
 
-function renderElement(
+function renderElementVNode(
   push: PushFn,
   vnode: VNode,
   parentComponent: ComponentInternalInstance
@@ -325,17 +342,17 @@ function renderElement(
   }
 }
 
-function renderPortal(
+function renderPortalVNode(
   vnode: VNode,
   parentComponent: ComponentInternalInstance
 ) {
   const target = vnode.props && vnode.props.target
   if (!target) {
-    console.warn(`[@vue/server-renderer] Portal is missing target prop.`)
+    console.error(`[@vue/server-renderer] Portal is missing target prop.`)
     return []
   }
   if (!isString(target)) {
-    console.warn(
+    console.error(
       `[@vue/server-renderer] Portal target must be a query selector string.`
     )
     return []
@@ -367,7 +384,7 @@ async function resolvePortals(context: SSRContext) {
   }
 }
 
-async function renderSuspense(
+async function renderSuspenseVNode(
   vnode: VNode,
   parentComponent: ComponentInternalInstance
 ): Promise<ResolvedSSRBuffer> {
@@ -375,10 +392,15 @@ async function renderSuspense(
   try {
     const { push, getBuffer } = createBuffer()
     renderVNode(push, content, parentComponent)
+    // await here so error can be caught
     return await getBuffer()
-  } catch {
-    const { push, getBuffer } = createBuffer()
-    renderVNode(push, fallback, parentComponent)
-    return getBuffer()
+  } catch (e) {
+    if (e[AsyncSetupErrorMarker]) {
+      const { push, getBuffer } = createBuffer()
+      renderVNode(push, fallback, parentComponent)
+      return getBuffer()
+    } else {
+      throw e
+    }
   }
 }