From db0e1554ba5df1748fee829ad5ad6fd4ee15b82d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafa=C5=82=20Ch=C5=82odnicki?= <rchl2k@gmail.com>
Date: Tue, 23 May 2023 01:10:38 +0200
Subject: [PATCH] fix(vue-app): skip page render early on error or navigation
 (#20719)

---
 packages/vue-app/template/client.js           | 59 +++++++++++++---
 packages/vue-app/template/utils.js            | 10 ++-
 test/e2e/basic.navigation.test.js             | 68 +++++++++++++++++++
 test/fixtures/basic/middleware/slow.js        |  3 +
 test/fixtures/basic/pages/index.vue           |  3 +
 test/fixtures/basic/pages/navigation.vue      | 27 ++++++++
 .../pages/navigation/asyncData-error.vue      | 11 +++
 .../pages/navigation/asyncData-throw.vue      | 11 +++
 .../navigation/middleware-asyncData-error.vue | 11 +++
 .../navigation/middleware-asyncData-throw.vue | 11 +++
 10 files changed, 202 insertions(+), 12 deletions(-)
 create mode 100644 test/e2e/basic.navigation.test.js
 create mode 100644 test/fixtures/basic/middleware/slow.js
 create mode 100644 test/fixtures/basic/pages/navigation.vue
 create mode 100644 test/fixtures/basic/pages/navigation/asyncData-error.vue
 create mode 100644 test/fixtures/basic/pages/navigation/asyncData-throw.vue
 create mode 100644 test/fixtures/basic/pages/navigation/middleware-asyncData-error.vue
 create mode 100644 test/fixtures/basic/pages/navigation/middleware-asyncData-throw.vue

diff --git a/packages/vue-app/template/client.js b/packages/vue-app/template/client.js
index 4e70ab31e8..7610117298 100644
--- a/packages/vue-app/template/client.js
+++ b/packages/vue-app/template/client.js
@@ -244,7 +244,7 @@ function resolveComponents (route) {
 <% } %>
 
 <% if (features.middleware) { %>
-function callMiddleware (Components, context, layout) {
+function callMiddleware (Components, context, layout, renderState) {
   let midd = <%= devalue(router.middleware) %><%= isTest ? '// eslint-disable-line' : '' %>
   let unknownMiddleware = false
 
@@ -278,7 +278,7 @@ function callMiddleware (Components, context, layout) {
   if (unknownMiddleware) {
     return
   }
-  return middlewareSeries(midd, context)
+  return middlewareSeries(midd, context, renderState)
 }
 <% } else if (isDev) {
 // This is a placeholder function mainly so we dont have to
@@ -288,7 +288,7 @@ function callMiddleware () {
   return Promise.resolve(true)
 }
 <% } %>
-async function render (to, from, next) {
+async function render (to, from, next, renderState) {
   if (this._routeChanged === false && this._paramChanged === false && this._queryChanged === false) {
     return next()
   }
@@ -329,6 +329,12 @@ async function render (to, from, next) {
   await setContext(app, {
     route: to,
     from,
+    error: (err) => {
+      if (renderState.aborted) {
+        return
+      }
+      app.nuxt.error.call(this, err)
+    },
     next: _next.bind(this)
   })
   this._dateLastError = app.nuxt.dateErr
@@ -342,10 +348,14 @@ async function render (to, from, next) {
   if (!Components.length) {
     <% if (features.middleware) { %>
     // Default layout
-    await callMiddleware.call(this, Components, app.context)
+    await callMiddleware.call(this, Components, app.context, undefined, renderState)
     if (nextCalled) {
       return
     }
+    if (renderState.aborted) {
+      next(false)
+      return
+    }
     <% } %>
 
     <% if (features.layouts) { %>
@@ -359,10 +369,14 @@ async function render (to, from, next) {
     <% } %>
 
     <% if (features.middleware) { %>
-    await callMiddleware.call(this, Components, app.context, layout)
+    await callMiddleware.call(this, Components, app.context, layout, renderState)
     if (nextCalled) {
       return
     }
+    if (renderState.aborted) {
+      next(false)
+      return
+    }
     <% } %>
 
     // Show error page
@@ -387,10 +401,14 @@ async function render (to, from, next) {
   try {
     <% if (features.middleware) { %>
     // Call middleware
-    await callMiddleware.call(this, Components, app.context)
+    await callMiddleware.call(this, Components, app.context, undefined, renderState)
     if (nextCalled) {
       return
     }
+    if (renderState.aborted) {
+      next(false)
+      return
+    }
     if (app.context._errored) {
       return next()
     }
@@ -407,10 +425,14 @@ async function render (to, from, next) {
 
     <% if (features.middleware) { %>
     // Call middleware for layout
-    await callMiddleware.call(this, Components, app.context, layout)
+    await callMiddleware.call(this, Components, app.context, layout, renderState)
     if (nextCalled) {
       return
     }
+    if (renderState.aborted) {
+      next(false)
+      return
+    }
     if (app.context._errored) {
       return next()
     }
@@ -578,10 +600,18 @@ async function render (to, from, next) {
         this.$loading.finish()
       }
       <% } %>
+      if (renderState.aborted) {
+        next(false)
+        return
+      }
       next()
     }
 
   } catch (err) {
+    if (renderState.aborted) {
+      next(false)
+      return
+    }
     const error = err || {}
     if (error.message === 'ERR_REDIRECT') {
       return this.<%= globals.nuxt %>.$emit('routeChanged', to, from, error)
@@ -908,7 +938,17 @@ async function mountApp (__app) {
 
   // Add beforeEach router hooks
   router.beforeEach(loadAsyncComponents.bind(_app))
-  router.beforeEach(render.bind(_app))
+
+  // Each new invocation of render() aborts previous invocation
+  let renderState = null
+  const boundRender = render.bind(_app)
+  router.beforeEach((to, from, next) => {
+    if (renderState) {
+      renderState.aborted = true
+    }
+    renderState = { aborted: false }
+    boundRender(to, from, next, renderState)
+  })
 
   // Fix in static: remove trailing slash to force hydration
   // Full static, if server-rendered: hydrate, to allow custom redirect to generated page
@@ -954,5 +994,6 @@ async function mountApp (__app) {
         errorHandler(err)
       }
     })
-  })
+  },
+  { aborted: false })
 }
diff --git a/packages/vue-app/template/utils.js b/packages/vue-app/template/utils.js
index 99a4ad6b11..60046bb500 100644
--- a/packages/vue-app/template/utils.js
+++ b/packages/vue-app/template/utils.js
@@ -279,6 +279,10 @@ export async function setContext (app, context) {
     app.context.from = fromRouteData
   }
 
+  if (context.error) {
+    app.context.error = context.error
+  }
+
   app.context.next = context.next
   app.context._redirected = false
   app.context._errored = false
@@ -287,13 +291,13 @@ export async function setContext (app, context) {
   app.context.query = app.context.route.query || {}
 }
 <% if (features.middleware) { %>
-export function middlewareSeries (promises, appContext) {
-  if (!promises.length || appContext._redirected || appContext._errored) {
+export function middlewareSeries (promises, appContext, renderState) {
+  if (!promises.length || appContext._redirected || appContext._errored || (renderState && renderState.aborted)) {
     return Promise.resolve()
   }
   return promisify(promises[0], appContext)
     .then(() => {
-      return middlewareSeries(promises.slice(1), appContext)
+      return middlewareSeries(promises.slice(1), appContext, renderState)
     })
 }
 <% } %>
diff --git a/test/e2e/basic.navigation.test.js b/test/e2e/basic.navigation.test.js
new file mode 100644
index 0000000000..bc11a07c1e
--- /dev/null
+++ b/test/e2e/basic.navigation.test.js
@@ -0,0 +1,68 @@
+import Browser from '../utils/browser'
+import { loadFixture, getPort, Nuxt, waitFor } from '../utils'
+
+let port
+const browser = new Browser()
+const url = route => 'http://localhost:' + port + route
+
+let nuxt = null
+let page = null
+
+describe('basic browser navigation', () => {
+  beforeAll(async () => {
+    const config = await loadFixture('basic')
+    nuxt = new Nuxt(config)
+    await nuxt.ready()
+
+    port = await getPort()
+    await nuxt.server.listen(port, 'localhost')
+
+    await browser.start({
+      // slowMo: 50,
+      // headless: false
+    })
+  })
+
+  const TEST_PATHS = [
+    'asyncData-error',
+    'asyncData-throw',
+    'middleware-asyncData-error',
+    'middleware-asyncData-throw'
+  ]
+
+  for (const path of TEST_PATHS) {
+    test(`Navigate away from /navigation/${path}`, async () => {
+      page = await browser.page(url('/'))
+      expect(await page.$text('h1')).toBe('Index page')
+
+      await page.nuxt.navigate('/navigation')
+      expect(await page.$text('h3')).toContain('Click one of the links below')
+
+      await page.nuxt.navigate(`/navigation/${path}`, /* waitEnd */false)
+      await waitFor(nuxt.options.loading.throttle + 100)
+      const loading = await page.nuxt.loadingData()
+      expect(loading.show).toBe(true)
+
+      await page.nuxt.go(-1)
+      await page.waitForFunction(
+        '$nuxt.$loading.$data.show === false'
+      )
+      expect(await page.$text('h1')).toBe('Index page')
+
+      await waitFor(2000)
+
+      expect(await page.$text('h1')).toBe('Index page')
+    })
+  }
+
+  // Close server and ask nuxt to stop listening to file changes
+  afterAll(async () => {
+    await nuxt.close()
+  })
+
+  // Stop browser
+  afterAll(async () => {
+    await page.close()
+    await browser.close()
+  })
+})
diff --git a/test/fixtures/basic/middleware/slow.js b/test/fixtures/basic/middleware/slow.js
new file mode 100644
index 0000000000..b7f3828c62
--- /dev/null
+++ b/test/fixtures/basic/middleware/slow.js
@@ -0,0 +1,3 @@
+export default async function () {
+  await new Promise(resolve => setTimeout(resolve, 2000))
+}
diff --git a/test/fixtures/basic/pages/index.vue b/test/fixtures/basic/pages/index.vue
index 3519a5e4cf..d0ba0becef 100644
--- a/test/fixtures/basic/pages/index.vue
+++ b/test/fixtures/basic/pages/index.vue
@@ -2,5 +2,8 @@
   <div>
     <h1>Index page</h1>
     <img src="~/assets/test.svg">
+    <nuxt-link to="navigation">
+      Continue to /navigation
+    </nuxt-link>
   </div>
 </template>
diff --git a/test/fixtures/basic/pages/navigation.vue b/test/fixtures/basic/pages/navigation.vue
new file mode 100644
index 0000000000..24beec8183
--- /dev/null
+++ b/test/fixtures/basic/pages/navigation.vue
@@ -0,0 +1,27 @@
+<template>
+  <div>
+    <h3>Click one of the links below and navigate back in history before the page has fully loaded. Pass if no error page shown.</h3>
+    <ul>
+      <li v-for="test in tests" :key="test">
+        <nuxt-link :to="'navigation/' + test">
+          {{ test }}
+        </nuxt-link>
+      </li>
+    </ul>
+  </div>
+</template>
+
+<script>
+export default {
+  computed: {
+    tests () {
+      return [
+        'asyncData-error',
+        'asyncData-throw',
+        'middleware-asyncData-error',
+        'middleware-asyncData-throw'
+      ]
+    }
+  }
+}
+</script>
diff --git a/test/fixtures/basic/pages/navigation/asyncData-error.vue b/test/fixtures/basic/pages/navigation/asyncData-error.vue
new file mode 100644
index 0000000000..c1086f8741
--- /dev/null
+++ b/test/fixtures/basic/pages/navigation/asyncData-error.vue
@@ -0,0 +1,11 @@
+<template>
+  <h1>Async Data Error</h1>
+</template>
+<script>
+export default {
+  async asyncData ({ error }) {
+    await new Promise(resolve => setTimeout(resolve, 2000))
+    error({ statusCode: 401, message: 'Not allowed here' })
+  }
+}
+</script>
diff --git a/test/fixtures/basic/pages/navigation/asyncData-throw.vue b/test/fixtures/basic/pages/navigation/asyncData-throw.vue
new file mode 100644
index 0000000000..34833eed2f
--- /dev/null
+++ b/test/fixtures/basic/pages/navigation/asyncData-throw.vue
@@ -0,0 +1,11 @@
+<template>
+  <h1>Async Throw Error</h1>
+</template>
+<script>
+export default {
+  async asyncData () {
+    await new Promise(resolve => setTimeout(resolve, 2000))
+    throw new Error('crash')
+  }
+}
+</script>
diff --git a/test/fixtures/basic/pages/navigation/middleware-asyncData-error.vue b/test/fixtures/basic/pages/navigation/middleware-asyncData-error.vue
new file mode 100644
index 0000000000..7138ac2dca
--- /dev/null
+++ b/test/fixtures/basic/pages/navigation/middleware-asyncData-error.vue
@@ -0,0 +1,11 @@
+<template>
+  <h1>Async Data Error</h1>
+</template>
+<script>
+export default {
+  middleware: ['slow'],
+  asyncData ({ error, route }) {
+    error({ statusCode: 401, message: 'Not allowed here' })
+  }
+}
+</script>
diff --git a/test/fixtures/basic/pages/navigation/middleware-asyncData-throw.vue b/test/fixtures/basic/pages/navigation/middleware-asyncData-throw.vue
new file mode 100644
index 0000000000..aa503ab654
--- /dev/null
+++ b/test/fixtures/basic/pages/navigation/middleware-asyncData-throw.vue
@@ -0,0 +1,11 @@
+<template>
+  <h1>Async Throw Error</h1>
+</template>
+<script>
+export default {
+  middleware: ['slow'],
+  asyncData () {
+    throw new Error('crash')
+  }
+}
+</script>