Fix custom 500 page rendering for render-time errors with experimental.advancedRouting astro/hono handlers#17041
Conversation
🦋 Changeset detectedLatest commit: c83263c The changes in this PR will be included in the next version bump. This PR includes changesets to release 400 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // so routeData is only missing when the custom 404 page is | ||
| // prerendered (or absent) — let the error handler serve the | ||
| // pre-built page instead. | ||
| if (!state.routeData) { |
There was a problem hiding this comment.
I don't see why this is needed. The problem is 500s I thought? 404s are already handled a level up (in App that calls into the fetch handler).
There was a problem hiding this comment.
This covers the case where the custom 404 is prerendered (or missing) - since #16911 routeData stays unset there, and pages() has no AstroHandler above it to turn that into a 404.
That said, I rebased on latest main and reworked this branch to return the X-Astro-Error response instead (same thing handle() does for the un-dispatched redirect case), so it's now handled by App's post-check a level up - which I think is the shape you were after. The 500 path still calls renderError directly, since the post-check can't carry the error object and 500.astro would lose its error prop. Tests still pass, I tightened the prerendered-404 one to assert the marker.
…l.advancedRouting astro/hono handlers The composable `astro/fetch` `pages()` entry point dispatched to `PagesHandler.handle()` without any error handling, so a page that threw during render propagated the error to the host framework (Hono returned its default plain-text "Internal Server Error") instead of rendering the custom `500.astro` page like the standard `AstroHandler` path does. `pages()` now delegates to a new `PagesHandler.handleWithErrorFallback()` that mirrors `AstroHandler`'s app-level error handling: unmatched routes (possible when the custom 404 page is prerendered, see withastro#16911) render the 404 error page, and render-time errors are logged and render the 500 error page via `app.renderError()` (loop-safe; falls back to a plain response when the error page itself fails). Fixes withastro#16952
Review feedback: instead of calling `app.renderError()` from `handleWithErrorFallback()` when no route matched, return the `X-Astro-Error`-marked 404 response (the same pattern `handle()` uses for the un-dispatched redirect case) and let the app's existing post-check render the 404 error page a level up. The 500 path keeps calling `renderError()` directly because the marker cannot carry the error object: the post-check passes `error: null`, which would drop the `error` prop on `500.astro` and skip logging the stack.
cb2f1b3 to
3ca4beb
Compare
The changeset only described the 500 behavior. The PR also changes what happens when no route matches and the custom 404 page is prerendered (or absent): previously the TypeError escaped to the host framework's default error response, now the 404 error page is rendered.
Changes
Under
experimental.advancedRouting, the composableastro/honopages()handler delegated straight toPagesHandler.handle(), with none of the app-level error handling thatAstroHandlerprovides on the standard path. A page throwing during render therefore propagated to the host framework, which answered with its own default error response (Hono: plain-textInternal Server Error) instead of rendering the custom500.astropage (#16952).PagesHandler.handleWithErrorFallback(app, state)(core/pages/handler.ts): wraps route dispatch in a try/catch that logs the error and renders it viaapp.renderError(..., { status: 500, error }), the same behaviourAstroHandler.render()has on the standard path. Forwardingerrorkeeps the documentederrorprop on500.astroworking. The logic lives in the handler module becausecore/fetch/index.tsdeliberately keeps its exports as thin, logic-free wrappers.handleWithErrorFallback()returns a 404 response marked withX-Astro-Error(the same patternhandle()uses for the un-dispatched redirect case), so the app's existing post-check renders the 404 error page a level up. This case is reachable because Fix 404 route resolution for experimental.advancedRouting with astro/hono handlers #16911 intentionally leavesrouteDataunset when the custom 404 route is prerendered (or absent), wherePagesHandler.handle()would otherwise throw aTypeErrorand the catch would turn those unmatched requests into 500s. The 500 path keeps callingrenderError()directly: the marker cannot carry the error object, so500.astrowould lose itserrorprop and the stack would not be logged.pages()incore/fetch/index.tsto delegate to the new method (delegation only, no logic added).astropatch).Testing
'renders the custom 500 page when a page throws during render'- verifies the fullpages()pipeline returns HTTP 500 with the custom 500 page content when a page throws during render'returns a marked 404 for the app post-check when the custom 404 route is prerendered'- verifies unmatched requests return HTTP 404 carrying theX-Astro-Errormarker instead of throwing500.astronot rendered for render-time errors underexperimental.advancedRouting+astro/honopages()#16952 (Hono +app.use(pages()), node standalone):GET /boom->500with the custom 500 page (previously Hono's plain-textInternal Server Error),GET /does-not-exist->404serving the prerendered 404 page with the marker stripped from the final response,GET /->200; the thrown error is logged with its stack traceOut of scope (noticed while testing)
Errors thrown by Astro middleware itself (
src/middleware.ts) on the composable path (app.use(middleware())) still reach the host framework's default error handler instead of500.astro. On the standard path,AstroHandler.render()'s try/catch wraps the whole middleware chain, so middleware errors get the same500.astrotreatment. The equivalent catch can't simply live inside the composablemiddleware(): itsnextis the host'snext(), so it would also intercept errors thrown by host middleware running below it in the chain and hide them from the user's ownapp.onError. The all-in-oneastro()handler already provides full parity. A possible follow-up would be an error-page helper exported fromastro/hono(e.g.app.onError(onError())) so composable users can opt in - happy to open a separate issue for this.Docs
Closes #16952