fix(nuxt): improve handling of redirects within middleware (#20244)

packages/nuxt/src/app/composables/router.ts

@@ -8,7 +8,6 @@ import { useNuxtApp, useRuntimeConfig } from '../nuxt'
 import type { NuxtError } from './error'
 import { createError } from './error'
 import { useState } from './state'
-import { setResponseStatus } from './ssr'
 
 import type { PageMeta } from '#app'
 
@@ -87,7 +86,7 @@ export interface NavigateToOptions {
   external?: boolean
 }
 
-export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: NavigateToOptions): Promise<void | NavigationFailure> | RouteLocationRaw => {
+export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: NavigateToOptions): Promise<void | NavigationFailure | false> | RouteLocationRaw => {
   if (!to) {
     to = '/'
   }
@@ -101,8 +100,10 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: Na
     throw new Error('Cannot navigate to an URL with script protocol.')
   }
 
+  const inMiddleware = isProcessingMiddleware()
+
   // Early redirect on client-side
-  if (process.client && !isExternal && isProcessingMiddleware()) {
+  if (process.client && !isExternal && inMiddleware) {
     return to
   }
 
@@ -111,15 +112,19 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: Na
   if (process.server) {
     const nuxtApp = useNuxtApp()
     if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) {
-      // Let vue-router handle internal redirects within middleware
-      // to prevent the navigation happening after response is sent
-      if (isProcessingMiddleware() && !isExternal) {
-        setResponseStatus(nuxtApp.ssrContext.event, options?.redirectCode || 302)
+      const fullPath = typeof to === 'string' || isExternal ? toPath : router.resolve(to).fullPath || '/'
+      const redirectLocation = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, fullPath)
+      const redirect = () => nuxtApp.callHook('app:redirected')
+        .then(() => sendRedirect(nuxtApp.ssrContext!.event, redirectLocation, options?.redirectCode || 302))
+        .then(() => inMiddleware ? /* abort route navigation */ false : undefined)
+
+      // We wait to perform the redirect in case any other middleware will intercept the redirect
+      // and redirect further.
+      if (!isExternal && inMiddleware) {
+        router.beforeEach(final => (final.fullPath === fullPath) ? redirect() : undefined)
         return to
       }
-      const redirectLocation = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/')
-      return nuxtApp.callHook('app:redirected')
-        .then(() => sendRedirect(nuxtApp.ssrContext!.event, redirectLocation, options?.redirectCode || 302))
+      return redirect()
     }
   }
 

packages/nuxt/src/app/plugins/router.ts

@@ -246,6 +246,7 @@ export default defineNuxtPlugin<{ route: Route, router: Router }>({
                 statusCode: 404,
                 statusMessage: `Page Not Found: ${initialURL}`
               })
+              delete nuxtApp._processingMiddleware
               return callWithNuxt(nuxtApp, showError, [error])
             }
           }
@@ -253,9 +254,7 @@ export default defineNuxtPlugin<{ route: Route, router: Router }>({
         }
       })
 
-      router.afterEach(() => {
-        delete nuxtApp._processingMiddleware
-      })
+      router.afterEach(() => { delete nuxtApp._processingMiddleware })
 
       await router.replace(initialURL)
       if (!isEqual(route.fullPath, initialURL)) {

packages/nuxt/src/core/runtime/nitro/renderer.ts

@@ -247,6 +247,8 @@ export default defineRenderHandler(async (event) => {
   })
   await ssrContext.nuxt?.hooks.callHook('app:rendered', { ssrContext })
 
+  if (event.node.res.headersSent || event.node.res.writableEnded) { return }
+
   // Handle errors
   if (ssrContext.payload?.error && !ssrError) {
     throw ssrContext.payload.error

packages/nuxt/src/pages/runtime/plugins/router.ts

@@ -13,7 +13,6 @@ import { isEqual, withoutBase } from 'ufo'
 import type { PageMeta, Plugin, RouteMiddleware } from '../../../app/index'
 import { callWithNuxt, defineNuxtPlugin, useRuntimeConfig } from '#app/nuxt'
 import { clearError, showError, useError } from '#app/composables/error'
-import { useRequestEvent } from '#app/composables/ssr'
 import { useState } from '#app/composables/state'
 import { navigateTo } from '#app/composables/router'
 
@@ -164,13 +163,18 @@ export default defineNuxtPlugin({
       }
     })
 
-    router.afterEach(async (to) => {
+    router.onError(() => { delete nuxtApp._processingMiddleware })
+
+    router.afterEach(async (to, _from, failure) => {
       delete nuxtApp._processingMiddleware
 
       if (process.client && !nuxtApp.isHydrating && error.value) {
         // Clear any existing errors
         await callWithNuxt(nuxtApp, clearError)
       }
+      if (process.server && failure?.type === 4 /* ErrorTypes.NAVIGATION_ABORTED */) {
+        return
+      }
       if (to.matched.length === 0) {
         await callWithNuxt(nuxtApp, showError, [createError({
           statusCode: 404,
@@ -180,9 +184,7 @@ export default defineNuxtPlugin({
       } else if (process.server) {
         const currentURL = to.fullPath || '/'
         if (!isEqual(currentURL, initialURL, { trailingSlash: true })) {
-          const event = await callWithNuxt(nuxtApp, useRequestEvent)
-          const options = { redirectCode: event.node.res.statusCode !== 200 ? event.node.res.statusCode || 302 : 302 }
-          await callWithNuxt(nuxtApp, navigateTo, [currentURL, options])
+          await callWithNuxt(nuxtApp, navigateTo, [currentURL])
         }
       }
     })

test/basic.test.ts

@@ -513,6 +513,19 @@ describe('navigate', () => {
     expect(res.status).toEqual(307)
     expect(await res.text()).toMatchInlineSnapshot('"<!DOCTYPE html><html><head><meta http-equiv=\\"refresh\\" content=\\"0; url=/navigate-some-path\\"></head></html>"')
   })
+
+  it('should not overwrite headers', async () => {
+    const { headers, status } = await fetch('/navigate-to-external', { redirect: 'manual' })
+
+    expect(headers.get('location')).toEqual('/')
+    expect(status).toEqual(302)
+  })
+
+  it('supports directly aborting navigation on SSR', async () => {
+    const { status } = await fetch('/navigate-to-false', { redirect: 'manual' })
+
+    expect(status).toEqual(404)
+  })
 })
 
 describe('errors', () => {

test/bundle.test.ts

@@ -48,7 +48,7 @@ describe.skipIf(isWindows || process.env.TEST_BUILDER === 'webpack' || process.e
 
   it('default server bundle size', async () => {
     stats.server = await analyzeSizes(['**/*.mjs', '!node_modules'], serverDir)
-    expect(roundToKilobytes(stats.server.totalBytes)).toMatchInlineSnapshot('"92.5k"')
+    expect(roundToKilobytes(stats.server.totalBytes)).toMatchInlineSnapshot('"92.6k"')
 
     const modules = await analyzeSizes('node_modules/**/*', serverDir)
     expect(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot('"2650k"')

test/fixtures/basic/middleware/redirect.global.ts

@@ -9,6 +9,12 @@ export default defineNuxtRouteMiddleware(async (to) => {
     await new Promise(resolve => setTimeout(resolve, 100))
     return navigateTo(to.path.slice('/redirect/'.length - 1))
   }
+  if (to.path === '/navigate-to-external') {
+    return navigateTo('/', { external: true })
+  }
+  if (to.path === '/navigate-to-false') {
+    return false
+  }
   const pluginPath = nuxtApp.$path()
   if (process.server && !/redirect|navigate/.test(pluginPath) && to.path !== pluginPath) {
     throw new Error('plugin did not run before middleware')

test/fixtures/basic/pages/[...slug].vue

@@ -11,4 +11,7 @@ definePageMeta({
   middleware: ['override'],
   validate: to => to.path !== '/forbidden'
 })
+if (useRoute().path.includes('navigate-some-path')) {
+  throw createError('navigate-some-path setup running')
+}
 </script>

test/fixtures/basic/server/plugins/headers.ts

@@ -0,0 +1,13 @@
+
+export default defineNitroPlugin((nitroApp) => {
+  if (!process.dev) { return }
+
+  const onError = nitroApp.h3App.options.onError!
+  nitroApp.h3App.options.onError = (error, event) => {
+    // TODO: somehow add error logging assertion to @nuxt/test-utils
+    if (error.message?.includes('Cannot set headers after they are sent to the client')) {
+      process.exit(1)
+    }
+    return onError(error, event)
+  }
+})

test/fixtures/basic/types.ts

@@ -97,7 +97,7 @@ describe('middleware', () => {
     addRouteMiddleware('example', (to, from) => {
       expectTypeOf(to).toEqualTypeOf<RouteLocationNormalizedLoaded>()
       expectTypeOf(from).toEqualTypeOf<RouteLocationNormalizedLoaded>()
-      expectTypeOf(navigateTo).toEqualTypeOf<(to: RouteLocationRaw | null | undefined, options?: NavigateToOptions) => RouteLocationRaw | Promise<void | NavigationFailure>>()
+      expectTypeOf(navigateTo).toEqualTypeOf<(to: RouteLocationRaw | null | undefined, options?: NavigateToOptions) => RouteLocationRaw | Promise<void | NavigationFailure | false>>()
       navigateTo('/')
       abortNavigation()
       abortNavigation('error string')
@@ -253,7 +253,7 @@ describe('composables', () => {
       .toEqualTypeOf(useLazyAsyncData(() => Promise.resolve({ foo: Math.random() }), { transform: data => data.foo }))
 
     // Default values: #14437
-    expectTypeOf(useAsyncData('test', () => Promise.resolve({ foo: { bar: 500 } }), { default: () => ({ bar: 500 }), transform: v => v.foo }).data).toEqualTypeOf<Ref<{bar: number} | null>>()
+    expectTypeOf(useAsyncData('test', () => Promise.resolve({ foo: { bar: 500 } }), { default: () => ({ bar: 500 }), transform: v => v.foo }).data).toEqualTypeOf<Ref<{ bar: number } | null>>()
     expectTypeOf(useLazyAsyncData('test', () => Promise.resolve({ foo: { bar: 500 } }), { default: () => ({ bar: 500 }), transform: v => v.foo }))
       .toEqualTypeOf(useLazyAsyncData(() => Promise.resolve({ foo: { bar: 500 } }), { default: () => ({ bar: 500 }), transform: v => v.foo }))
     expectTypeOf(useFetch('/api/hey', { default: () => 'bar', transform: v => v.foo }).data).toEqualTypeOf<Ref<string | null>>()