From ca0986e35fac583402411f568402ba4b0c7d2918 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 29 Apr 2023 21:54:49 +0100 Subject: [PATCH 1/9] test: add test for setting cookie in plugin on 404 --- packages/nuxt/src/app/composables/cookie.ts | 5 ++--- test/basic.test.ts | 12 ++++++++---- test/fixtures/basic/plugins/cookie.ts | 3 +++ 3 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/basic/plugins/cookie.ts diff --git a/packages/nuxt/src/app/composables/cookie.ts b/packages/nuxt/src/app/composables/cookie.ts index a578b85e4975..3d588cc76ed0 100644 --- a/packages/nuxt/src/app/composables/cookie.ts +++ b/packages/nuxt/src/app/composables/cookie.ts @@ -48,11 +48,10 @@ export function useCookie (name: string, _opts?: } } const unhook = nuxtApp.hooks.hookOnce('app:rendered', writeFinalCookieValue) - const writeAndUnhook = () => { + nuxtApp.hooks.hookOnce('app:error', () => { unhook() // don't write cookie subsequently when app:rendered is called return writeFinalCookieValue() - } - nuxtApp.hooks.hookOnce('app:error', writeAndUnhook) + }) } return cookie as CookieRef diff --git a/test/basic.test.ts b/test/basic.test.ts index 0755a888cf2e..c2203c5f79b9 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -114,8 +114,9 @@ describe('pages', () => { }) it('validates routes', async () => { - const { status } = await fetch('/forbidden') + const { status, headers } = await fetch('/forbidden') expect(status).toEqual(404) + expect(headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/') const page = await createPage('/navigate-to-forbidden') await page.waitForLoadState('networkidle') @@ -135,8 +136,11 @@ describe('pages', () => { expect(status).toEqual(500) }) - it('render 404', async () => { - const html = await $fetch('/not-found') + it('render catchall page', async () => { + const res = await fetch('/not-found') + expect(res.status).toEqual(200) + + const html = await res.text() // Snapshot // expect(html).toMatchInlineSnapshot() @@ -578,7 +582,7 @@ describe('errors', () => { it('should render a HTML error page', async () => { const res = await fetch('/error') - expect(res.headers.get('Set-Cookie')).toBe('some-error=was%20set; Path=/') + expect(res.headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/, some-error=was%20set; Path=/') expect(await res.text()).toContain('This is a custom error') }) diff --git a/test/fixtures/basic/plugins/cookie.ts b/test/fixtures/basic/plugins/cookie.ts new file mode 100644 index 000000000000..8e969548c996 --- /dev/null +++ b/test/fixtures/basic/plugins/cookie.ts @@ -0,0 +1,3 @@ +export default defineNuxtPlugin(() => { + useCookie('set-in-plugin').value = 'true' +}) From 3302b550c4f20771d171df0af0426003444d270c Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 29 Apr 2023 23:14:30 +0100 Subject: [PATCH 2/9] fix(nuxt): ensure `useError` is called with nuxt app context --- packages/nuxt/src/app/composables/error.ts | 3 ++- test/bundle.test.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nuxt/src/app/composables/error.ts b/packages/nuxt/src/app/composables/error.ts index 3d1820f139c9..e6114a1ff591 100644 --- a/packages/nuxt/src/app/composables/error.ts +++ b/packages/nuxt/src/app/composables/error.ts @@ -13,8 +13,9 @@ export const showError = (_err: string | Error | Partial) => { try { const nuxtApp = useNuxtApp() - nuxtApp.callHook('app:error', err) const error = useError() + const p = nuxtApp.hooks.callHook('app:error', err) + if (process.server) { nuxtApp.hook('app:created', () => p) } error.value = error.value || err } catch { throw err diff --git a/test/bundle.test.ts b/test/bundle.test.ts index eab3032392e6..0bf9f4693671 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -45,7 +45,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('"66.8k"') + expect(roundToKilobytes(stats.server.totalBytes)).toMatchInlineSnapshot('"66.9k"') const modules = await analyzeSizes('node_modules/**/*', serverDir) expect(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot('"2654k"') From 931188a87220568e1051e5d1729f8f94c5e2e18c Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sun, 30 Apr 2023 22:57:27 +0100 Subject: [PATCH 3/9] fix: only call `app:error` once on server` --- packages/nuxt/src/app/composables/error.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nuxt/src/app/composables/error.ts b/packages/nuxt/src/app/composables/error.ts index e6114a1ff591..b23fcd32ee57 100644 --- a/packages/nuxt/src/app/composables/error.ts +++ b/packages/nuxt/src/app/composables/error.ts @@ -14,8 +14,9 @@ export const showError = (_err: string | Error | Partial) => { try { const nuxtApp = useNuxtApp() const error = useError() - const p = nuxtApp.hooks.callHook('app:error', err) - if (process.server) { nuxtApp.hook('app:created', () => p) } + if (process.client) { + nuxtApp.hooks.callHook('app:error', err) + } error.value = error.value || err } catch { throw err From d1771fe10dc0ca1a8629bd47faca1505f116354b Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 1 May 2023 00:24:16 +0100 Subject: [PATCH 4/9] fix: append cookies from error page, not replace --- packages/nuxt/src/core/runtime/nitro/error.ts | 6 ++++-- test/basic.test.ts | 4 ++-- test/bundle.test.ts | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/nuxt/src/core/runtime/nitro/error.ts b/packages/nuxt/src/core/runtime/nitro/error.ts index 80ce8c8b1d4c..86ca55891207 100644 --- a/packages/nuxt/src/core/runtime/nitro/error.ts +++ b/packages/nuxt/src/core/runtime/nitro/error.ts @@ -1,7 +1,7 @@ import { joinURL, withQuery } from 'ufo' import type { NitroErrorHandler } from 'nitropack' import type { H3Error } from 'h3' -import { getRequestHeaders, setResponseHeader, setResponseStatus } from 'h3' +import { appendResponseHeader, getRequestHeaders, setResponseHeader, setResponseStatus } from 'h3' import { useNitroApp, useRuntimeConfig } from '#internal/nitro' import { isJsonRequest, normalizeError } from '#internal/nitro/utils' @@ -69,7 +69,9 @@ export default async function errorhandler (error: H3Error, } for (const [header, value] of res.headers.entries()) { - setResponseHeader(event, header, value) + // TODO: remove `appendResponseHeader` when we drop support for node 16 + const handler = header.toLowerCase() === 'set-cookie' ? appendResponseHeader : setResponseHeader + handler(event, header, value) } setResponseStatus(event, res.status && res.status !== 200 ? res.status : undefined, res.statusText) diff --git a/test/basic.test.ts b/test/basic.test.ts index c2203c5f79b9..841a4471d5ac 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -116,7 +116,7 @@ describe('pages', () => { it('validates routes', async () => { const { status, headers } = await fetch('/forbidden') expect(status).toEqual(404) - expect(headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/') + expect(headers.get('Set-Cookie')).toContain('set-in-plugin=true; Path=/') const page = await createPage('/navigate-to-forbidden') await page.waitForLoadState('networkidle') @@ -582,7 +582,7 @@ describe('errors', () => { it('should render a HTML error page', async () => { const res = await fetch('/error') - expect(res.headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/, some-error=was%20set; Path=/') + expect(res.headers.get('Set-Cookie')).toContain('set-in-plugin=true; Path=/, some-error=was%20set; Path=/') expect(await res.text()).toContain('This is a custom error') }) diff --git a/test/bundle.test.ts b/test/bundle.test.ts index 9c5fb0586848..09ad2ef4a203 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -45,7 +45,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('"66.6k"') + expect(roundToKilobytes(stats.server.totalBytes)).toMatchInlineSnapshot('"66.7k"') const modules = await analyzeSizes('node_modules/**/*', serverDir) expect(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot('"2654k"') From 777708c6fdde58097ac246c3ba35e2a1bdc3e2fe Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 1 May 2023 23:22:56 +0100 Subject: [PATCH 5/9] refactor: prefer `appendResponseHeader` --- docs/1.getting-started/6.data-fetching.md | 4 ++-- packages/nuxt/src/app/components/nuxt-island.ts | 4 ++-- packages/nuxt/src/app/composables/cookie.ts | 4 ++-- packages/nuxt/src/components/runtime/server-component.ts | 4 ++-- packages/nuxt/src/core/runtime/nitro/renderer.ts | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/1.getting-started/6.data-fetching.md b/docs/1.getting-started/6.data-fetching.md index 7800cb16d3f4..69f1da51f221 100644 --- a/docs/1.getting-started/6.data-fetching.md +++ b/docs/1.getting-started/6.data-fetching.md @@ -314,13 +314,13 @@ Be very careful before proxying headers to an external API and just include head If you want to pass on/proxy cookies in the other direction, from an internal request back to the client, you will need to handle this yourself. ```ts [composables/fetch.ts] -import { appendHeader, H3Event } from 'h3' +import { appendResponseHeader, H3Event } from 'h3' export const fetchWithCookie = async (event: H3Event, url: string) => { const res = await $fetch.raw(url) const cookies = (res.headers.get('set-cookie') || '').split(',') for (const cookie of cookies) { - appendHeader(event, 'set-cookie', cookie) + appendResponseHeader(event, 'set-cookie', cookie) } return res._data } diff --git a/packages/nuxt/src/app/components/nuxt-island.ts b/packages/nuxt/src/app/components/nuxt-island.ts index 69ebb5e523d6..ea7892398419 100644 --- a/packages/nuxt/src/app/components/nuxt-island.ts +++ b/packages/nuxt/src/app/components/nuxt-island.ts @@ -2,7 +2,7 @@ import type { RendererNode } from 'vue' import { computed, createStaticVNode, defineComponent, getCurrentInstance, h, ref, watch } from 'vue' import { debounce } from 'perfect-debounce' import { hash } from 'ohash' -import { appendHeader } from 'h3' +import { appendResponseHeader } from 'h3' import { useHead } from '@unhead/vue' // eslint-disable-next-line import/no-restricted-paths @@ -42,7 +42,7 @@ export default defineComponent({ const url = `/__nuxt_island/${props.name}:${hashId.value}` if (process.server && process.env.prerender) { // Hint to Nitro to prerender the island component - appendHeader(event, 'x-nitro-prerender', url) + appendResponseHeader(event, 'x-nitro-prerender', url) } // TODO: Validate response return $fetch(url, { diff --git a/packages/nuxt/src/app/composables/cookie.ts b/packages/nuxt/src/app/composables/cookie.ts index 3d588cc76ed0..a05773cd7941 100644 --- a/packages/nuxt/src/app/composables/cookie.ts +++ b/packages/nuxt/src/app/composables/cookie.ts @@ -2,7 +2,7 @@ import type { Ref } from 'vue' import { ref, watch } from 'vue' import type { CookieParseOptions, CookieSerializeOptions } from 'cookie-es' import { parse, serialize } from 'cookie-es' -import { appendHeader } from 'h3' +import { appendResponseHeader } from 'h3' import type { H3Event } from 'h3' import destr from 'destr' import { isEqual } from 'ohash' @@ -81,6 +81,6 @@ function writeClientCookie (name: string, value: any, opts: CookieSerializeOptio function writeServerCookie (event: H3Event, name: string, value: any, opts: CookieSerializeOptions = {}) { if (event) { // TODO: Try to smart join with existing Set-Cookie headers - appendHeader(event, 'Set-Cookie', serializeCookie(name, value, opts)) + appendResponseHeader(event, 'Set-Cookie', serializeCookie(name, value, opts)) } } diff --git a/packages/nuxt/src/components/runtime/server-component.ts b/packages/nuxt/src/components/runtime/server-component.ts index bef75caf4a3f..cdd9e69d4764 100644 --- a/packages/nuxt/src/components/runtime/server-component.ts +++ b/packages/nuxt/src/components/runtime/server-component.ts @@ -1,7 +1,7 @@ import { Fragment, computed, createStaticVNode, createVNode, defineComponent, h, ref, watch } from 'vue' import { debounce } from 'perfect-debounce' import { hash } from 'ohash' -import { appendHeader } from 'h3' +import { appendResponseHeader } from 'h3' import { useHead } from '@unhead/vue' import type { NuxtIslandResponse } from '../../core/runtime/nitro/renderer' @@ -51,7 +51,7 @@ const NuxtServerComponent = defineComponent({ const url = `/__nuxt_island/${props.name}:${hashId.value}` if (process.server && process.env.prerender) { // Hint to Nitro to prerender the island component - appendHeader(event, 'x-nitro-prerender', url) + appendResponseHeader(event, 'x-nitro-prerender', url) } // TODO: Validate response return $fetch(url, { diff --git a/packages/nuxt/src/core/runtime/nitro/renderer.ts b/packages/nuxt/src/core/runtime/nitro/renderer.ts index 0ec0d5d4be1b..f997c85acd83 100644 --- a/packages/nuxt/src/core/runtime/nitro/renderer.ts +++ b/packages/nuxt/src/core/runtime/nitro/renderer.ts @@ -2,7 +2,7 @@ import { createRenderer, renderResourceHeaders } from 'vue-bundle-renderer/runti import type { RenderResponse } from 'nitropack' import type { Manifest } from 'vite' import type { H3Event } from 'h3' -import { appendHeader, createError, getQuery, readBody, writeEarlyHints } from 'h3' +import { appendResponseHeader, createError, getQuery, readBody, writeEarlyHints } from 'h3' import devalue from '@nuxt/devalue' import { stringify, uneval } from 'devalue' import destr from 'destr' @@ -275,7 +275,7 @@ export default defineRenderHandler(async (event): Promise Date: Mon, 1 May 2023 23:28:13 +0100 Subject: [PATCH 6/9] fix: revert change and add note to docs --- docs/1.getting-started/8.error-handling.md | 4 ++++ packages/nuxt/src/core/runtime/nitro/error.ts | 4 +--- test/bundle.test.ts | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/1.getting-started/8.error-handling.md b/docs/1.getting-started/8.error-handling.md index e340967ec113..eeb004d2f17e 100644 --- a/docs/1.getting-started/8.error-handling.md +++ b/docs/1.getting-started/8.error-handling.md @@ -67,6 +67,10 @@ When you are ready to remove the error page, you can call the `clearError` helpe Make sure to check before using anything dependent on Nuxt plugins, such as `$route` or `useRouter`, as if a plugin threw an error, then it won't be re-run until you clear the error. :: +::alert{type="warning"} +If you are running on Node 16 and you set any cookies when rendering your error page, they will [overwrite cookies previously set](https://github.com/nuxt/nuxt/pull/20585). We recommend using a newer version of Node as Node 16 will reach end-of-life in September 2023. +:: + ### Example ```vue [error.vue] diff --git a/packages/nuxt/src/core/runtime/nitro/error.ts b/packages/nuxt/src/core/runtime/nitro/error.ts index 86ca55891207..60b7390af62d 100644 --- a/packages/nuxt/src/core/runtime/nitro/error.ts +++ b/packages/nuxt/src/core/runtime/nitro/error.ts @@ -69,9 +69,7 @@ export default async function errorhandler (error: H3Error, } for (const [header, value] of res.headers.entries()) { - // TODO: remove `appendResponseHeader` when we drop support for node 16 - const handler = header.toLowerCase() === 'set-cookie' ? appendResponseHeader : setResponseHeader - handler(event, header, value) + setResponseHeader(event, header, value) } setResponseStatus(event, res.status && res.status !== 200 ? res.status : undefined, res.statusText) diff --git a/test/bundle.test.ts b/test/bundle.test.ts index 09ad2ef4a203..9c5fb0586848 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -45,7 +45,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('"66.7k"') + expect(roundToKilobytes(stats.server.totalBytes)).toMatchInlineSnapshot('"66.6k"') const modules = await analyzeSizes('node_modules/**/*', serverDir) expect(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot('"2654k"') From 53cfbeeafcffe74554e4d5aec9d5f674a577a965 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 1 May 2023 23:32:01 +0100 Subject: [PATCH 7/9] test: temporarily disable cookie snapshot --- test/basic.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/basic.test.ts b/test/basic.test.ts index 841a4471d5ac..0e95b90a9dea 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -116,7 +116,7 @@ describe('pages', () => { it('validates routes', async () => { const { status, headers } = await fetch('/forbidden') expect(status).toEqual(404) - expect(headers.get('Set-Cookie')).toContain('set-in-plugin=true; Path=/') + expect(headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/') const page = await createPage('/navigate-to-forbidden') await page.waitForLoadState('networkidle') @@ -582,7 +582,8 @@ describe('errors', () => { it('should render a HTML error page', async () => { const res = await fetch('/error') - expect(res.headers.get('Set-Cookie')).toContain('set-in-plugin=true; Path=/, some-error=was%20set; Path=/') + // TODO: enable when we update test to node v16 + // expect(res.headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/, some-error=was%20set; Path=/') expect(await res.text()).toContain('This is a custom error') }) From a66895d959f61b0e929dc15aa8bdeaeb87ac5bfb Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 1 May 2023 23:34:57 +0100 Subject: [PATCH 8/9] chore: remove unneeded import --- packages/nuxt/src/core/runtime/nitro/error.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nuxt/src/core/runtime/nitro/error.ts b/packages/nuxt/src/core/runtime/nitro/error.ts index 60b7390af62d..80ce8c8b1d4c 100644 --- a/packages/nuxt/src/core/runtime/nitro/error.ts +++ b/packages/nuxt/src/core/runtime/nitro/error.ts @@ -1,7 +1,7 @@ import { joinURL, withQuery } from 'ufo' import type { NitroErrorHandler } from 'nitropack' import type { H3Error } from 'h3' -import { appendResponseHeader, getRequestHeaders, setResponseHeader, setResponseStatus } from 'h3' +import { getRequestHeaders, setResponseHeader, setResponseStatus } from 'h3' import { useNitroApp, useRuntimeConfig } from '#internal/nitro' import { isJsonRequest, normalizeError } from '#internal/nitro/utils' From 712eadb84ee746b312f4c3ba43f9a7a67bc65bfb Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 1 May 2023 23:41:02 +0100 Subject: [PATCH 9/9] test: add assertion to trigger updating test --- test/basic.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/basic.test.ts b/test/basic.test.ts index 0e95b90a9dea..8f15f25096f3 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -582,6 +582,7 @@ describe('errors', () => { it('should render a HTML error page', async () => { const res = await fetch('/error') + expect(res.headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/') // TODO: enable when we update test to node v16 // expect(res.headers.get('Set-Cookie')).toBe('set-in-plugin=true; Path=/, some-error=was%20set; Path=/') expect(await res.text()).toContain('This is a custom error')