Skip to content

Commit

Permalink
fix: fix and improve error HTTP response (fix #1872)
Browse files Browse the repository at this point in the history
  • Loading branch information
brillout committed Sep 11, 2024
1 parent 253353e commit 0634732
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
26 changes: 20 additions & 6 deletions vike/node/runtime/renderPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ import { handleErrorWithoutErrorPage } from './renderPage/handleErrorWithoutErro
import { loadUserFilesServerSide } from './renderPage/loadUserFilesServerSide.js'
import { resolveRedirects } from './renderPage/resolveRedirects.js'
import { PageContextBuiltInServerInternal } from '../../shared/types.js'
import type { PageFile } from '../../shared/getPageFiles.js'
import type { PageConfigRuntime } from '../../shared/page-configs/PageConfig.js'

const globalObject = getGlobalObject('runtime/renderPage.ts', {
httpRequestsCount: 0
Expand Down Expand Up @@ -124,7 +126,7 @@ async function renderPageAndPrepare(
httpRequestId,
'error'
)
const pageContextWithError = getPageContextHttpResponseError(err, pageContextInit)
const pageContextWithError = getPageContextHttpResponseError(err, pageContextInit, null)
return pageContextWithError
}
if (isConfigInvalid) {
Expand All @@ -145,7 +147,7 @@ async function renderPageAndPrepare(
// initGlobalContext_renderPage() and getRenderContext() don't call any user hooks => err isn't thrown from user code.
assert(!isAbortError(err))
logRuntimeError(err, httpRequestId)
const pageContextWithError = getPageContextHttpResponseError(err, pageContextInit)
const pageContextWithError = getPageContextHttpResponseError(err, pageContextInit, null)
return pageContextWithError
}
if (isConfigInvalid) {
Expand Down Expand Up @@ -298,7 +300,11 @@ async function renderPageAlreadyPrepared(
)} doesn't occur while the error page is being rendered.`,
{ onlyOnce: false }
)
const pageContextHttpWithError = getPageContextHttpResponseError(errNominalPage, pageContextInit)
const pageContextHttpWithError = getPageContextHttpResponseError(
errNominalPage,
pageContextInit,
pageContextErrorPageInit
)
return pageContextHttpWithError
}
// `throw redirect()` / `throw render(url)`
Expand All @@ -307,7 +313,11 @@ async function renderPageAlreadyPrepared(
if (isNewError(errErrorPage, errNominalPage)) {
logRuntimeError(errErrorPage, httpRequestId)
}
const pageContextWithError = getPageContextHttpResponseError(errNominalPage, pageContextInit)
const pageContextWithError = getPageContextHttpResponseError(
errNominalPage,
pageContextInit,
pageContextErrorPageInit
)
return pageContextWithError
}
return pageContextErrorPage
Expand Down Expand Up @@ -365,10 +375,14 @@ function prettyUrl(url: string) {

function getPageContextHttpResponseError(
err: unknown,
pageContextInit: Record<string, unknown>
pageContextInit: Record<string, unknown>,
pageContext: null | {
_pageFilesAll: PageFile[]
_pageConfigs: PageConfigRuntime[]
}
): PageContextAfterRender {
const pageContextWithError = createPageContext(pageContextInit)
const httpResponse = createHttpResponseError()
const httpResponse = createHttpResponseError(pageContext)
objectAssign(pageContextWithError, {
httpResponse,
errorWhileRendering: err
Expand Down
25 changes: 22 additions & 3 deletions vike/node/runtime/renderPage/createHttpResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import type { GetPageAssets } from './getPageAssets.js'
import { assert, assertWarning } from '../utils.js'
import type { HtmlRender } from '../html/renderHtml.js'
import type { PageConfigRuntime } from '../../../shared/page-configs/PageConfig.js'
import { isErrorPage } from '../../../shared/error-page.js'
import { getErrorPageId, isErrorPage } from '../../../shared/error-page.js'
import type { RenderHook } from './executeOnRenderHtmlHook.js'
import type { RedirectStatusCode, AbortStatusCode, UrlRedirect } from '../../../shared/route/abort.js'
import { getHttpResponseBody, getHttpResponseBodyStreamHandlers, HttpResponseBody } from './getHttpResponseBody.js'
import { getEarlyHints, type EarlyHint } from './getEarlyHints.js'
import { getCacheControl } from './createHttpResponse/getCacheControl.js'
import { assertNoInfiniteHttpRedirect } from './createHttpResponse/assertNoInfiniteHttpRedirect.js'
import type { PageFile } from '../../../shared/getPageFiles.js'

type HttpResponse = {
statusCode: 200 | 404 | 500 | RedirectStatusCode | AbortStatusCode
Expand Down Expand Up @@ -80,12 +81,30 @@ function createHttpResponseFavicon404(): HttpResponse {
return httpResponse
}

function createHttpResponseError(): HttpResponse {
function createHttpResponseError(
pageContext: null | {
_pageFilesAll: PageFile[]
_pageConfigs: PageConfigRuntime[]
}
): HttpResponse {
const reason: string = (() => {
if (!pageContext) {
return 'no error page (https://vike.dev/error-page) could be rendered'
}
const errorPageId = getErrorPageId(pageContext._pageFilesAll, pageContext._pageConfigs)
if (errorPageId) {
return "the error page (https://vike.dev/error-page) couldn't be rendered (for example if an error occurred while rendering the error page)"
} else {
return 'no error page (https://vike.dev/error-page) is defined, make sure to create one'
}
})()
const httpResponse = createHttpResponse(
500,
'text/html;charset=utf-8',
[],
`<p>An error occurred.</p><script>console.log('This HTTP response was generated by Vike. This response is used instead of rendering the error page (https://vike.dev/error-page), either because there isn\'t error page or because an error occurred while rendering the error page.')</script>`
`<p>An error occurred.</p><script>console.log(${JSON.stringify(
`This HTTP response was generated by Vike. Vike returned this response because ${reason}.`
)})</script>`
)
return httpResponse
}
Expand Down
4 changes: 3 additions & 1 deletion vike/node/runtime/renderPage/handleErrorWithoutErrorPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import pc from '@brillout/picocolors'
import type { GetPageAssets } from './getPageAssets.js'
import type { PageContextAfterRender } from './renderPageAlreadyRouted.js'
import type { PageConfigRuntime } from '../../../shared/page-configs/PageConfig.js'
import type { PageFile } from '../../../shared/getPageFiles.js'

// When the user hasn't defined _error.page.js
async function handleErrorWithoutErrorPage<
Expand All @@ -16,6 +17,7 @@ async function handleErrorWithoutErrorPage<
errorWhileRendering: null | Error
is404: null | boolean
_pageId: null
_pageFilesAll: PageFile[]
_pageConfigs: PageConfigRuntime[]
urlOriginal: string
}
Expand All @@ -29,7 +31,7 @@ async function handleErrorWithoutErrorPage<
}

if (!pageContext.isClientSideNavigation) {
const httpResponse = createHttpResponseError()
const httpResponse = createHttpResponseError(pageContext)
objectAssign(pageContext, { httpResponse })
return pageContext
} else {
Expand Down

0 comments on commit 0634732

Please sign in to comment.