From 7a92b766f5023f86e8ed79d11f999b5bfc5fbb91 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Thu, 6 Mar 2025 14:17:46 +0000 Subject: [PATCH] feat(nuxt): align nuxt error handling with nitro (#31230) Co-authored-by: Daniel Philip Johnson Co-authored-by: Saeid Zareie --- eslint.config.mjs | 2 +- packages/nuxt/.gitignore | 4 +- packages/nuxt/src/app/composables/error.ts | 5 +- .../src/core/runtime/nitro/handlers/error.ts | 80 ++++++++----------- .../src/core/runtime/nitro/utils/error.ts | 27 +++++++ test/basic.test.ts | 14 +++- test/bundle.test.ts | 4 +- 7 files changed, 79 insertions(+), 57 deletions(-) create mode 100644 packages/nuxt/src/core/runtime/nitro/utils/error.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 61a0b25a6c..32e5a02ab9 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -21,7 +21,7 @@ export default createConfigForNuxt({ 'packages/schema/schema/**', 'packages/nuxt/src/app/components/welcome.vue', 'packages/nuxt/src/app/components/error-*.vue', - 'packages/nuxt/src/core/runtime/nitro/handlers/error-*', + 'packages/nuxt/src/core/runtime/nitro/templates/error-*', ], }, { diff --git a/packages/nuxt/.gitignore b/packages/nuxt/.gitignore index afa5e6a973..3a60aa6317 100644 --- a/packages/nuxt/.gitignore +++ b/packages/nuxt/.gitignore @@ -2,5 +2,5 @@ src/app/components/error-404.vue src/app/components/error-500.vue src/app/components/error-dev.vue src/app/components/welcome.vue -src/core/runtime/nitro/handlers/error-500.ts -src/core/runtime/nitro/handlers/error-dev.ts +src/core/runtime/nitro/templates/error-500.ts +src/core/runtime/nitro/templates/error-dev.ts diff --git a/packages/nuxt/src/app/composables/error.ts b/packages/nuxt/src/app/composables/error.ts index af164622f6..e9ff815cdb 100644 --- a/packages/nuxt/src/app/composables/error.ts +++ b/packages/nuxt/src/app/composables/error.ts @@ -14,8 +14,9 @@ export const NUXT_ERROR_SIGNATURE = '__nuxt_error' /** @since 3.0.0 */ export const useError = (): Ref => toRef(useNuxtApp().payload, 'error') -// eslint-disable-next-line @typescript-eslint/no-empty-object-type -export interface NuxtError extends H3Error {} +export interface NuxtError extends H3Error { + error?: true +} /** @since 3.0.0 */ export const showError = ( diff --git a/packages/nuxt/src/core/runtime/nitro/handlers/error.ts b/packages/nuxt/src/core/runtime/nitro/handlers/error.ts index de7e89192d..792163320a 100644 --- a/packages/nuxt/src/core/runtime/nitro/handlers/error.ts +++ b/packages/nuxt/src/core/runtime/nitro/handlers/error.ts @@ -1,53 +1,40 @@ import { joinURL, withQuery } from 'ufo' import type { NitroErrorHandler } from 'nitropack' -import type { H3Error } from 'h3' -import { getRequestHeaders, send, setResponseHeader, setResponseStatus } from 'h3' - -import type { NuxtPayload } from 'nuxt/app' +import { getRequestHeaders, send, setResponseHeader, setResponseHeaders, setResponseStatus } from 'h3' +import { isJsonRequest } from '../utils/error' import { useRuntimeConfig } from '#internal/nitro' import { useNitroApp } from '#internal/nitro/app' -import { isJsonRequest, normalizeError } from '#internal/nitro/utils' +import type { NuxtPayload } from '#app/nuxt' -export default async function errorhandler (error: H3Error, event) { - // Parse and normalize error - const { stack, statusCode, statusMessage, message } = normalizeError(error) - - // Create an error object - const errorObject = { - url: event.path, - statusCode, - statusMessage, - message, - stack: import.meta.dev && statusCode !== 404 - ? `
${stack.map(i => `${i.text}`).join('\n')}
` - : '', - // TODO: check and validate error.data for serialisation into query - data: error.data as any, - } satisfies Partial & { url: string } - - // Console output - if (error.unhandled || error.fatal) { - const tags = [ - '[nuxt]', - '[request error]', - error.unhandled && '[unhandled]', - error.fatal && '[fatal]', - Number(errorObject.statusCode) !== 200 && `[${errorObject.statusCode}]`, - ].filter(Boolean).join(' ') - console.error(tags, (error.message || error.toString() || 'internal server error') + '\n' + stack.map(l => ' ' + l.text).join(' \n')) - } - - if (event.handled) { return } - - // Set response code and message - setResponseStatus(event, (errorObject.statusCode !== 200 && errorObject.statusCode) as any as number || 500, errorObject.statusMessage) - - // JSON response +export default async function errorhandler (error, event, { defaultHandler }) { if (isJsonRequest(event)) { - setResponseHeader(event, 'Content-Type', 'application/json') - return send(event, JSON.stringify(errorObject)) + // let Nitro handle JSON errors + return } + // invoke default Nitro error handler (which will log appropriately if required) + const defaultRes = await defaultHandler(error, event, { json: true }) + + // let Nitro handle redirect if appropriate + const statusCode = error.statusCode || 500 + if (statusCode === 404 && defaultRes.status === 302) { + setResponseHeaders(event, defaultRes.headers) + setResponseStatus(event, defaultRes.status, defaultRes.statusText) + return send(event, JSON.stringify(defaultRes.body, null, 2)) + } + + if (import.meta.dev && typeof defaultRes.body !== 'string' && Array.isArray(defaultRes.body.stack)) { + // normalize to string format expected by nuxt `error.vue` + defaultRes.body.stack = defaultRes.body.stack.join('\n') + } + + const errorObject = defaultRes.body as Pick, 'error' | 'statusCode' | 'statusMessage' | 'message' | 'stack'> & { url: string, data: any } + errorObject.message ||= 'Server Error' + + delete defaultRes.headers['content-type'] // this would be set to application/json + delete defaultRes.headers['content-security-policy'] // this would disable JS execution in the error page + + setResponseHeaders(event, defaultRes.headers) // Access request headers const reqHeaders = getRequestHeaders(event) @@ -66,25 +53,24 @@ export default async function errorhandler (error: H3Error, }, ).catch(() => null) + if (event.handled) { return } + // Fallback to static rendered error page if (!res) { - const { template } = import.meta.dev ? await import('./error-dev') : await import('./error-500') + const { template } = import.meta.dev ? await import('../templates/error-dev') : await import('../templates/error-500') if (import.meta.dev) { // TODO: Support `message` in template (errorObject as any).description = errorObject.message } - if (event.handled) { return } setResponseHeader(event, 'Content-Type', 'text/html;charset=UTF-8') return send(event, template(errorObject)) } const html = await res.text() - if (event.handled) { return } - for (const [header, value] of res.headers.entries()) { setResponseHeader(event, header, value) } - setResponseStatus(event, res.status && res.status !== 200 ? res.status : undefined, res.statusText) + setResponseStatus(event, res.status && res.status !== 200 ? res.status : defaultRes.status, res.statusText || defaultRes.statusText) return send(event, html) } diff --git a/packages/nuxt/src/core/runtime/nitro/utils/error.ts b/packages/nuxt/src/core/runtime/nitro/utils/error.ts new file mode 100644 index 0000000000..448c341c70 --- /dev/null +++ b/packages/nuxt/src/core/runtime/nitro/utils/error.ts @@ -0,0 +1,27 @@ +import { type H3Event, getRequestHeader } from 'h3' + +/** + * Nitro internal functions extracted from https://github.com/nitrojs/nitro/blob/main/src/runtime/internal/utils.ts + */ + +export function isJsonRequest (event: H3Event) { + // If the client specifically requests HTML, then avoid classifying as JSON. + if (hasReqHeader(event, 'accept', 'text/html')) { + return false + } + return ( + hasReqHeader(event, 'accept', 'application/json') || + hasReqHeader(event, 'user-agent', 'curl/') || + hasReqHeader(event, 'user-agent', 'httpie/') || + hasReqHeader(event, 'sec-fetch-mode', 'cors') || + event.path.startsWith('/api/') || + event.path.endsWith('.json') + ) +} + +export function hasReqHeader (event: H3Event, name: string, includes: string) { + const value = getRequestHeader(event, name) + return ( + value && typeof value === 'string' && value.toLowerCase().includes(includes) + ) +} diff --git a/test/basic.test.ts b/test/basic.test.ts index fd273cd9f6..2050331800 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -1166,11 +1166,14 @@ describe('errors', () => { expect(res.statusText).toBe('This is a custom error') const error = await res.json() delete error.stack + const url = new URL(error.url) + url.host = 'localhost:3000' + error.url = url.toString() expect(error).toMatchObject({ - message: 'This is a custom error', + message: isDev() ? 'This is a custom error' : 'Server Error', statusCode: 422, statusMessage: 'This is a custom error', - url: '/error', + url: 'http://localhost:3000/error', }) }) @@ -1189,12 +1192,17 @@ describe('errors', () => { expect(res.status).toBe(404) const error = await res.json() delete error.stack + const url = new URL(error.url) + url.host = 'localhost:3000' + error.url = url.toString() + expect(error).toMatchInlineSnapshot(` { + "error": true, "message": "Page Not Found: /__nuxt_error", "statusCode": 404, "statusMessage": "Page Not Found: /__nuxt_error", - "url": "/__nuxt_error", + "url": "http://localhost:3000/__nuxt_error", } `) }) diff --git a/test/bundle.test.ts b/test/bundle.test.ts index 4e692329c7..c6e2e34f46 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -37,7 +37,7 @@ describe.skipIf(process.env.SKIP_BUNDLE_SIZE === 'true' || process.env.ECOSYSTEM const serverDir = join(rootDir, '.output/server') const serverStats = await analyzeSizes(['**/*.mjs', '!node_modules'], serverDir) - expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot(`"193k"`) + expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot(`"192k"`) const modules = await analyzeSizes(['node_modules/**/*'], serverDir) expect.soft(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot(`"1384k"`) @@ -74,7 +74,7 @@ describe.skipIf(process.env.SKIP_BUNDLE_SIZE === 'true' || process.env.ECOSYSTEM const serverDir = join(rootDir, '.output-inline/server') const serverStats = await analyzeSizes(['**/*.mjs', '!node_modules'], serverDir) - expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot(`"544k"`) + expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot(`"543k"`) const modules = await analyzeSizes(['node_modules/**/*'], serverDir) expect.soft(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot(`"77.8k"`)