summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Emanuele Stoppa <my.burning@gmail.com> 2024-06-05 13:09:51 +0100
committerGravatar GitHub <noreply@github.com> 2024-06-05 13:09:51 +0100
commit35ef53c0897c0d360efc086a71c5f4406721d2fe (patch)
tree9000904414d7b24aa0d167897ce8bedafaa7c343
parent6bcbd15f86cddeb45b982e9d12fb2da35b9ff0e1 (diff)
downloadastro-35ef53c0897c0d360efc086a71c5f4406721d2fe.tar.gz
astro-35ef53c0897c0d360efc086a71c5f4406721d2fe.tar.zst
astro-35ef53c0897c0d360efc086a71c5f4406721d2fe.zip
fix: bubble up errors in rewrites (#11136)
* fix: bubble up errors in rewrites * docs * remove commented code * changesets * fix string interpolation
-rw-r--r--.changeset/dull-lizards-jog.md5
-rw-r--r--.changeset/nervous-pens-retire.md7
-rw-r--r--packages/astro/src/container/pipeline.ts11
-rw-r--r--packages/astro/src/core/app/pipeline.ts56
-rw-r--r--packages/astro/src/core/base-pipeline.ts17
-rw-r--r--packages/astro/src/core/build/generate.ts2
-rw-r--r--packages/astro/src/core/build/pipeline.ts54
-rw-r--r--packages/astro/src/core/constants.ts2
-rw-r--r--packages/astro/src/core/errors/errors-data.ts23
-rw-r--r--packages/astro/src/core/render-context.ts112
-rw-r--r--packages/astro/src/core/routing/astro-designed-error-pages.ts44
-rw-r--r--packages/astro/src/vite-plugin-astro-server/pipeline.ts60
-rw-r--r--packages/astro/src/vite-plugin-astro-server/response.ts14
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts3
-rw-r--r--packages/astro/test/fixtures/reroute/src/pages/blog/oops.astro11
-rw-r--r--packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs9
-rw-r--r--packages/astro/test/fixtures/rewrite-404-invalid/package.json8
-rw-r--r--packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro3
-rw-r--r--packages/astro/test/fixtures/rewrite-runtime-error/astro.config.mjs9
-rw-r--r--packages/astro/test/fixtures/rewrite-runtime-error/package.json8
-rw-r--r--packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/from.astro4
-rw-r--r--packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/to.astro4
-rw-r--r--packages/astro/test/rewrite.test.js49
-rw-r--r--pnpm-lock.yaml12
24 files changed, 360 insertions, 167 deletions
diff --git a/.changeset/dull-lizards-jog.md b/.changeset/dull-lizards-jog.md
new file mode 100644
index 000000000..9f5d7bdb2
--- /dev/null
+++ b/.changeset/dull-lizards-jog.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Errors that are emitted during a rewrite are now bubbled up and shown to the user. A 404 response is not returned anymore.
diff --git a/.changeset/nervous-pens-retire.md b/.changeset/nervous-pens-retire.md
new file mode 100644
index 000000000..7815d37ad
--- /dev/null
+++ b/.changeset/nervous-pens-retire.md
@@ -0,0 +1,7 @@
+---
+"astro": patch
+---
+
+It's not possible anymore to use `Astro.rewrite("/404")` inside static pages. This isn't counterproductive because Astro will end-up emitting a page that contains the HTML of 404 error page.
+
+It's still possible to use `Astro.rewrite("/404")` inside on-demand pages, or pages that opt-out from prerendering.
diff --git a/packages/astro/src/container/pipeline.ts b/packages/astro/src/container/pipeline.ts
index d48d13e42..5dae72b64 100644
--- a/packages/astro/src/container/pipeline.ts
+++ b/packages/astro/src/container/pipeline.ts
@@ -7,12 +7,13 @@ import type {
} from '../@types/astro.js';
import { type HeadElements, Pipeline } from '../core/base-pipeline.js';
import type { SinglePageBuiltModule } from '../core/build/types.js';
-import { RouteNotFound } from '../core/errors/errors-data.js';
+import { InvalidRewrite404, RouteNotFound } from '../core/errors/errors-data.js';
import { AstroError } from '../core/errors/index.js';
import {
createModuleScriptElement,
createStylesheetElementSet,
} from '../core/render/ssr-element.js';
+import { default404Page } from '../core/routing/astro-designed-error-pages.js';
export class ContainerPipeline extends Pipeline {
/**
@@ -111,4 +112,12 @@ export class ContainerPipeline extends Pipeline {
// At the moment it's not used by the container via any public API
// @ts-expect-error It needs to be implemented.
async getComponentByRoute(_routeData: RouteData): Promise<ComponentInstance> {}
+
+ rewriteKnownRoute(pathname: string, _sourceRoute: RouteData): ComponentInstance {
+ if (pathname === '/404') {
+ return { default: default404Page } as any as ComponentInstance;
+ }
+
+ throw new AstroError(InvalidRewrite404);
+ }
}
diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts
index 3eec8068f..b74cc68ee 100644
--- a/packages/astro/src/core/app/pipeline.ts
+++ b/packages/astro/src/core/app/pipeline.ts
@@ -9,8 +9,11 @@ import type {
import { Pipeline } from '../base-pipeline.js';
import type { SinglePageBuiltModule } from '../build/types.js';
import { DEFAULT_404_COMPONENT } from '../constants.js';
+import { RewriteEncounteredAnError } from '../errors/errors-data.js';
+import { AstroError } from '../errors/index.js';
import { RedirectSinglePageBuiltModule } from '../redirects/component.js';
import { createModuleScriptElement, createStylesheetElementSet } from '../render/ssr-element.js';
+import { DEFAULT_404_ROUTE } from '../routing/astro-designed-error-pages.js';
export class AppPipeline extends Pipeline {
#manifestData: ManifestData | undefined;
@@ -82,36 +85,44 @@ export class AppPipeline extends Pipeline {
async tryRewrite(
payload: RewritePayload,
- request: Request
+ request: Request,
+ sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance]> {
let foundRoute;
for (const route of this.#manifestData!.routes) {
+ let finalUrl: URL | undefined = undefined;
+
if (payload instanceof URL) {
- if (route.pattern.test(payload.pathname)) {
- foundRoute = route;
- break;
- }
+ finalUrl = payload;
} else if (payload instanceof Request) {
- const url = new URL(payload.url);
- if (route.pattern.test(url.pathname)) {
- foundRoute = route;
- break;
- }
+ finalUrl = new URL(payload.url);
} else {
- const newUrl = new URL(payload, new URL(request.url).origin);
- if (route.pattern.test(decodeURI(newUrl.pathname))) {
- foundRoute = route;
- break;
- }
+ finalUrl = new URL(payload, new URL(request.url).origin);
+ }
+
+ if (route.pattern.test(decodeURI(finalUrl.pathname))) {
+ foundRoute = route;
+ break;
+ } else if (finalUrl.pathname === '/404') {
+ foundRoute = DEFAULT_404_ROUTE;
+ break;
}
}
if (foundRoute) {
- const componentInstance = await this.getComponentByRoute(foundRoute);
- return [foundRoute, componentInstance];
+ if (foundRoute.pathname === '/404') {
+ const componentInstance = this.rewriteKnownRoute(foundRoute.pathname, sourceRoute);
+ return [foundRoute, componentInstance];
+ } else {
+ const componentInstance = await this.getComponentByRoute(foundRoute);
+ return [foundRoute, componentInstance];
+ }
}
- throw new Error('Route not found');
+ throw new AstroError({
+ ...RewriteEncounteredAnError,
+ message: RewriteEncounteredAnError.message(payload.toString()),
+ });
}
async getModuleForRoute(route: RouteData): Promise<SinglePageBuiltModule> {
@@ -141,4 +152,13 @@ export class AppPipeline extends Pipeline {
);
}
}
+
+ // We don't need to check the source route, we already are in SSR
+ rewriteKnownRoute(pathname: string, _sourceRoute: RouteData): ComponentInstance {
+ if (pathname === '/404') {
+ return { default: () => new Response(null, { status: 404 }) } as ComponentInstance;
+ } else {
+ return { default: () => new Response(null, { status: 500 }) } as ComponentInstance;
+ }
+ }
}
diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts
index 533094bb2..e7baa0f2b 100644
--- a/packages/astro/src/core/base-pipeline.ts
+++ b/packages/astro/src/core/base-pipeline.ts
@@ -80,11 +80,14 @@ export abstract class Pipeline {
*
* - if not `RouteData` is found
*
- * @param {RewritePayload} rewritePayload
+ * @param {RewritePayload} rewritePayload The payload provided by the user
+ * @param {Request} request The original request
+ * @param {RouteData} sourceRoute The original `RouteData`
*/
abstract tryRewrite(
rewritePayload: RewritePayload,
- request: Request
+ request: Request,
+ sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance]>;
/**
@@ -92,6 +95,16 @@ export abstract class Pipeline {
* @param routeData
*/
abstract getComponentByRoute(routeData: RouteData): Promise<ComponentInstance>;
+
+ /**
+ * Attempts to execute a rewrite of a known Astro route:
+ * - /404 -> src/pages/404.astro -> template
+ * - /500 -> src/pages/500.astro
+ *
+ * @param pathname The pathname where the user wants to rewrite e.g. "/404"
+ * @param sourceRoute The original route where the first request started. This is needed in case a pipeline wants to check if the current route is pre-rendered or not
+ */
+ abstract rewriteKnownRoute(pathname: string, sourceRoute: RouteData): ComponentInstance;
}
// eslint-disable-next-line @typescript-eslint/no-empty-interface
diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
index e5bf622a9..56faa7a7c 100644
--- a/packages/astro/src/core/build/generate.ts
+++ b/packages/astro/src/core/build/generate.ts
@@ -44,8 +44,6 @@ import { getOutputFilename, isServerLikeOutput } from '../util.js';
import { getOutDirWithinCwd, getOutFile, getOutFolder } from './common.js';
import { cssOrder, mergeInlineCss } from './internal.js';
import { BuildPipeline } from './pipeline.js';
-import { ASTRO_PAGE_MODULE_ID } from './plugins/plugin-pages.js';
-import { getVirtualModulePageName } from './plugins/util.js';
import type {
PageBuildData,
SinglePageBuiltModule,
diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts
index 664d30c9a..59e4b2110 100644
--- a/packages/astro/src/core/build/pipeline.ts
+++ b/packages/astro/src/core/build/pipeline.ts
@@ -8,7 +8,7 @@ import type {
import { getOutputDirectory } from '../../prerender/utils.js';
import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
import type { SSRManifest } from '../app/types.js';
-import { RouteNotFound } from '../errors/errors-data.js';
+import { InvalidRewrite404, RewriteEncounteredAnError } from '../errors/errors-data.js';
import { AstroError } from '../errors/index.js';
import { routeIsFallback, routeIsRedirect } from '../redirects/helpers.js';
import { RedirectSinglePageBuiltModule } from '../redirects/index.js';
@@ -18,6 +18,7 @@ import {
createModuleScriptsSet,
createStylesheetElementSet,
} from '../render/ssr-element.js';
+import { DEFAULT_404_ROUTE } from '../routing/astro-designed-error-pages.js';
import { isServerLikeOutput } from '../util.js';
import { getOutDirWithinCwd } from './common.js';
import { type BuildInternals, cssOrder, getPageData, mergeInlineCss } from './internal.js';
@@ -278,35 +279,43 @@ export class BuildPipeline extends Pipeline {
async tryRewrite(
payload: RewritePayload,
- request: Request
+ request: Request,
+ sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance]> {
let foundRoute: RouteData | undefined;
// options.manifest is the actual type that contains the information
for (const route of this.options.manifest.routes) {
+ let finalUrl: URL | undefined = undefined;
+
if (payload instanceof URL) {
- if (route.pattern.test(payload.pathname)) {
- foundRoute = route;
- break;
- }
+ finalUrl = payload;
} else if (payload instanceof Request) {
- const url = new URL(payload.url);
- if (route.pattern.test(url.pathname)) {
- foundRoute = route;
- break;
- }
+ finalUrl = new URL(payload.url);
} else {
- const newUrl = new URL(payload, new URL(request.url).origin);
- if (route.pattern.test(decodeURI(newUrl.pathname))) {
- foundRoute = route;
- break;
- }
+ finalUrl = new URL(payload, new URL(request.url).origin);
+ }
+
+ if (route.pattern.test(decodeURI(finalUrl.pathname))) {
+ foundRoute = route;
+ break;
+ } else if (finalUrl.pathname === '/404') {
+ foundRoute = DEFAULT_404_ROUTE;
+ break;
}
}
if (foundRoute) {
- const componentInstance = await this.getComponentByRoute(foundRoute);
- return [foundRoute, componentInstance];
+ if (foundRoute.pathname === '/404') {
+ const componentInstance = await this.rewriteKnownRoute(foundRoute.pathname, sourceRoute);
+ return [foundRoute, componentInstance];
+ } else {
+ const componentInstance = await this.getComponentByRoute(foundRoute);
+ return [foundRoute, componentInstance];
+ }
} else {
- throw new AstroError(RouteNotFound);
+ throw new AstroError({
+ ...RewriteEncounteredAnError,
+ message: RewriteEncounteredAnError.message(payload.toString()),
+ });
}
}
@@ -367,6 +376,13 @@ export class BuildPipeline extends Pipeline {
return RedirectSinglePageBuiltModule;
}
+
+ rewriteKnownRoute(_pathname: string, sourceRoute: RouteData): ComponentInstance {
+ if (!isServerLikeOutput(this.config) || sourceRoute.prerender) {
+ throw new AstroError(InvalidRewrite404);
+ }
+ throw new Error(`Unreachable, in SSG this route shouldn't be generated`);
+ }
}
function createEntryURL(filePath: string, outFolder: URL) {
diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts
index 3347c1ec5..a4d32abe5 100644
--- a/packages/astro/src/core/constants.ts
+++ b/packages/astro/src/core/constants.ts
@@ -30,7 +30,7 @@ export const ROUTE_TYPE_HEADER = 'X-Astro-Route-Type';
/**
* The value of the `component` field of the default 404 page, which is used when there is no user-provided 404.astro page.
*/
-export const DEFAULT_404_COMPONENT = 'astro-default-404';
+export const DEFAULT_404_COMPONENT = 'astro-default-404.astro';
/**
* A response with one of these status codes will be rewritten
diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts
index 217ade537..1528d253f 100644
--- a/packages/astro/src/core/errors/errors-data.ts
+++ b/packages/astro/src/core/errors/errors-data.ts
@@ -1120,6 +1120,29 @@ export const MissingMiddlewareForInternationalization = {
/**
* @docs
* @description
+ * The user tried to rewrite using a route that doesn't exist, or it emitted a runtime error during its rendering phase.
+ */
+export const RewriteEncounteredAnError = {
+ name: 'RewriteEncounteredAnError',
+ title: "Astro couldn't find the route to rewrite, or if was found but it emitted an error during the rendering phase.",
+ message: (route: string, stack?: string) => `The route ${route} that you tried to render doesn't exist, or it emitted an error during the rendering phase. ${stack ? stack : ""}.`
+} satisfies ErrorData;
+
+/**
+ * @docs
+ * @description
+ *
+ * The user tried to rewrite a 404 page inside a static page.
+ */
+export const InvalidRewrite404 = {
+ name: 'InvalidRewrite404',
+ title: "You attempted to rewrite a 404 inside a static page, and this isn't allowed.",
+ message: 'Rewriting a 404 is only allowed inside on-demand pages.',
+} satisfies ErrorData;
+
+/**
+ * @docs
+ * @description
* Astro could not find an associated file with content while trying to render the route. This is an Astro error and not a user error. If restarting the dev server does not fix the problem, please file an issue.
*/
export const CantRenderPage = {
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index 22d259470..5a484446b 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -20,6 +20,7 @@ import { renderEndpoint } from '../runtime/server/endpoint.js';
import { renderPage } from '../runtime/server/index.js';
import {
ASTRO_VERSION,
+ DEFAULT_404_COMPONENT,
REROUTE_DIRECTIVE_HEADER,
ROUTE_TYPE_HEADER,
clientAddressSymbol,
@@ -38,6 +39,9 @@ import { type Pipeline, Slots, getParams, getProps } from './render/index.js';
* It contains data unique to each request. It is responsible for executing middleware, calling endpoints, and rendering the page by gathering necessary data from a `Pipeline`.
*/
export class RenderContext {
+ // The first route that this instance of the context attempts to render
+ originalRoute: RouteData;
+
private constructor(
readonly pipeline: Pipeline,
public locals: App.Locals,
@@ -50,7 +54,9 @@ export class RenderContext {
public params = getParams(routeData, pathname),
protected url = new URL(request.url),
public props: Props = {}
- ) {}
+ ) {
+ this.originalRoute = routeData;
+ }
/**
* A flag that tells the render content if the rewriting was triggered
@@ -124,18 +130,15 @@ export class RenderContext {
const lastNext = async (ctx: APIContext, payload?: RewritePayload) => {
if (payload) {
if (this.pipeline.manifest.rewritingEnabled) {
- try {
- const [routeData, component] = await pipeline.tryRewrite(payload, this.request);
- this.routeData = routeData;
- componentInstance = component;
- } catch (e) {
- return new Response('Not found', {
- status: 404,
- statusText: 'Not found',
- });
- } finally {
- this.isRewriting = true;
- }
+ // we intentionally let the error bubble up
+ const [routeData, component] = await pipeline.tryRewrite(
+ payload,
+ this.request,
+ this.originalRoute
+ );
+ this.routeData = routeData;
+ componentInstance = component;
+ this.isRewriting = true;
} else {
this.pipeline.logger.warn(
'router',
@@ -166,6 +169,7 @@ export class RenderContext {
result.cancelled = true;
throw e;
}
+
// Signal to the i18n middleware to maybe act on this response
response.headers.set(ROUTE_TYPE_HEADER, 'page');
// Signal to the error-page-rerouting infra to let this response pass through to avoid loops
@@ -218,29 +222,25 @@ export class RenderContext {
const rewrite = async (reroutePayload: RewritePayload) => {
pipeline.logger.debug('router', 'Called rewriting to:', reroutePayload);
- try {
- const [routeData, component] = await pipeline.tryRewrite(reroutePayload, this.request);
- this.routeData = routeData;
- if (reroutePayload instanceof Request) {
- this.request = reroutePayload;
- } else {
- this.request = this.#copyRequest(
- new URL(routeData.pathname ?? routeData.route, this.url.origin),
- this.request
- );
- }
- this.url = new URL(this.request.url);
- this.cookies = new AstroCookies(this.request);
- this.params = getParams(routeData, url.toString());
- this.isRewriting = true;
- return await this.render(component);
- } catch (e) {
- pipeline.logger.debug('router', 'Rewrite failed.', e);
- return new Response('Not found', {
- status: 404,
- statusText: 'Not found',
- });
+ const [routeData, component] = await pipeline.tryRewrite(
+ reroutePayload,
+ this.request,
+ this.originalRoute
+ );
+ this.routeData = routeData;
+ if (reroutePayload instanceof Request) {
+ this.request = reroutePayload;
+ } else {
+ this.request = this.#copyRequest(
+ new URL(routeData.pathname ?? routeData.route, this.url.origin),
+ this.request
+ );
}
+ this.url = new URL(this.request.url);
+ this.cookies = new AstroCookies(this.request);
+ this.params = getParams(routeData, url.toString());
+ this.isRewriting = true;
+ return await this.render(component);
};
return {
@@ -409,30 +409,26 @@ export class RenderContext {
};
const rewrite = async (reroutePayload: RewritePayload) => {
- try {
- pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload);
- const [routeData, component] = await pipeline.tryRewrite(reroutePayload, this.request);
- this.routeData = routeData;
- if (reroutePayload instanceof Request) {
- this.request = reroutePayload;
- } else {
- this.request = this.#copyRequest(
- new URL(routeData.pathname ?? routeData.route, this.url.origin),
- this.request
- );
- }
- this.url = new URL(this.request.url);
- this.cookies = new AstroCookies(this.request);
- this.params = getParams(routeData, url.toString());
- this.isRewriting = true;
- return await this.render(component);
- } catch (e) {
- pipeline.logger.debug('router', 'Rerouting failed, returning a 404.', e);
- return new Response('Not found', {
- status: 404,
- statusText: 'Not found',
- });
+ pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload);
+ const [routeData, component] = await pipeline.tryRewrite(
+ reroutePayload,
+ this.request,
+ this.originalRoute
+ );
+ this.routeData = routeData;
+ if (reroutePayload instanceof Request) {
+ this.request = reroutePayload;
+ } else {
+ this.request = this.#copyRequest(
+ new URL(routeData.pathname ?? routeData.route, this.url.origin),
+ this.request
+ );
}
+ this.url = new URL(this.request.url);
+ this.cookies = new AstroCookies(this.request);
+ this.params = getParams(routeData, url.toString());
+ this.isRewriting = true;
+ return await this.render(component);
};
return {
diff --git a/packages/astro/src/core/routing/astro-designed-error-pages.ts b/packages/astro/src/core/routing/astro-designed-error-pages.ts
index 1dbe7ed60..6a047f6d5 100644
--- a/packages/astro/src/core/routing/astro-designed-error-pages.ts
+++ b/packages/astro/src/core/routing/astro-designed-error-pages.ts
@@ -1,20 +1,38 @@
-import type { ManifestData } from '../../@types/astro.js';
+import type { ManifestData, RouteData } from '../../@types/astro.js';
+import notFoundTemplate from '../../template/4xx.js';
import { DEFAULT_404_COMPONENT } from '../constants.js';
+export const DEFAULT_404_ROUTE: RouteData = {
+ component: DEFAULT_404_COMPONENT,
+ generate: () => '',
+ params: [],
+ pattern: /\/404/,
+ prerender: false,
+ pathname: '/404',
+ segments: [[{ content: '404', dynamic: false, spread: false }]],
+ type: 'page',
+ route: '/404',
+ fallbackRoutes: [],
+ isIndex: false,
+};
+
export function ensure404Route(manifest: ManifestData) {
if (!manifest.routes.some((route) => route.route === '/404')) {
- manifest.routes.push({
- component: DEFAULT_404_COMPONENT,
- generate: () => '',
- params: [],
- pattern: /\/404/,
- prerender: false,
- segments: [[{ content: '404', dynamic: false, spread: false }]],
- type: 'page',
- route: '/404',
- fallbackRoutes: [],
- isIndex: false,
- });
+ manifest.routes.push(DEFAULT_404_ROUTE);
}
return manifest;
}
+
+export async function default404Page({ pathname }: { pathname: string }) {
+ return new Response(
+ notFoundTemplate({
+ statusCode: 404,
+ title: 'Not found',
+ tabTitle: '404: Not Found',
+ pathname,
+ }),
+ { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8' } }
+ );
+}
+// mark the function as an AstroComponentFactory for the rendering internals
+default404Page.isAstroComponentFactory = true;
diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts
index 260f7fa1f..66cce8961 100644
--- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts
+++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts
@@ -11,20 +11,20 @@ import type {
SSRManifest,
} from '../@types/astro.js';
import { getInfoOutput } from '../cli/info/index.js';
-import type { HeadElements } from '../core/base-pipeline.js';
+import { type HeadElements } from '../core/base-pipeline.js';
import { ASTRO_VERSION, DEFAULT_404_COMPONENT } from '../core/constants.js';
import { enhanceViteSSRError } from '../core/errors/dev/index.js';
-import { RouteNotFound } from '../core/errors/errors-data.js';
+import { InvalidRewrite404, RewriteEncounteredAnError } from '../core/errors/errors-data.js';
import { AggregateError, AstroError, CSSError, MarkdownError } from '../core/errors/index.js';
import type { Logger } from '../core/logger/core.js';
import type { ModuleLoader } from '../core/module-loader/index.js';
import { Pipeline, loadRenderer } from '../core/render/index.js';
+import { DEFAULT_404_ROUTE, default404Page } from '../core/routing/astro-designed-error-pages.js';
import { isPage, isServerLikeOutput, resolveIdToUrl, viteID } from '../core/util.js';
import { PAGE_SCRIPT_ID } from '../vite-plugin-scripts/index.js';
import { getStylesForURL } from './css.js';
import { getComponentMetadata } from './metadata.js';
import { createResolve } from './resolve.js';
-import { default404Page } from './response.js';
import { getScriptsForURL } from './scripts.js';
export class DevPipeline extends Pipeline {
@@ -193,7 +193,8 @@ export class DevPipeline extends Pipeline {
async tryRewrite(
payload: RewritePayload,
- request: Request
+ request: Request,
+ sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance]> {
let foundRoute;
if (!this.manifestData) {
@@ -201,35 +202,52 @@ export class DevPipeline extends Pipeline {
}
for (const route of this.manifestData.routes) {
+ let finalUrl: URL | undefined = undefined;
+
if (payload instanceof URL) {
- if (route.pattern.test(payload.pathname)) {
- foundRoute = route;
- break;
- }
+ finalUrl = payload;
} else if (payload instanceof Request) {
- const url = new URL(payload.url);
- if (route.pattern.test(url.pathname)) {
- foundRoute = route;
- break;
- }
+ finalUrl = new URL(payload.url);
} else {
- const newUrl = new URL(payload, new URL(request.url).origin);
- if (route.pattern.test(decodeURI(newUrl.pathname))) {
- foundRoute = route;
- break;
- }
+ finalUrl = new URL(payload, new URL(request.url).origin);
+ }
+
+ if (route.pattern.test(decodeURI(finalUrl.pathname))) {
+ foundRoute = route;
+ break;
+ } else if (finalUrl.pathname === '/404') {
+ foundRoute = DEFAULT_404_ROUTE;
+ break;
}
}
if (foundRoute) {
- const componentInstance = await this.getComponentByRoute(foundRoute);
- return [foundRoute, componentInstance];
+ if (foundRoute.pathname === '/404') {
+ const componentInstance = this.rewriteKnownRoute(foundRoute.pathname, sourceRoute);
+ return [foundRoute, componentInstance];
+ } else {
+ const componentInstance = await this.getComponentByRoute(foundRoute);
+ return [foundRoute, componentInstance];
+ }
} else {
- throw new AstroError(RouteNotFound);
+ throw new AstroError({
+ ...RewriteEncounteredAnError,
+ message: RewriteEncounteredAnError.message(payload.toString()),
+ });
}
}
setManifestData(manifestData: ManifestData) {
this.manifestData = manifestData;
}
+
+ rewriteKnownRoute(route: string, sourceRoute: RouteData): ComponentInstance {
+ if (isServerLikeOutput(this.config) && sourceRoute.prerender) {
+ if (route === '/404') {
+ return { default: default404Page } as any as ComponentInstance;
+ }
+ }
+
+ throw new AstroError(InvalidRewrite404);
+ }
}
diff --git a/packages/astro/src/vite-plugin-astro-server/response.ts b/packages/astro/src/vite-plugin-astro-server/response.ts
index bdf0e3c15..c6e034aef 100644
--- a/packages/astro/src/vite-plugin-astro-server/response.ts
+++ b/packages/astro/src/vite-plugin-astro-server/response.ts
@@ -23,20 +23,6 @@ export async function handle404Response(
writeHtmlResponse(res, 404, html);
}
-export async function default404Page({ pathname }: { pathname: string }) {
- return new Response(
- notFoundTemplate({
- statusCode: 404,
- title: 'Not found',
- tabTitle: '404: Not Found',
- pathname,
- }),
- { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8' } }
- );
-}
-// mark the function as an AstroComponentFactory for the rendering internals
-default404Page.isAstroComponentFactory = true;
-
export async function handle500Response(
loader: ModuleLoader,
res: http.ServerResponse,
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
index 95dd98135..b451a8ff6 100644
--- a/packages/astro/src/vite-plugin-astro-server/route.ts
+++ b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -11,11 +11,12 @@ import { loadMiddleware } from '../core/middleware/loadMiddleware.js';
import { RenderContext } from '../core/render-context.js';
import { type SSROptions, getProps } from '../core/render/index.js';
import { createRequest } from '../core/request.js';
+import { default404Page } from '../core/routing/astro-designed-error-pages.js';
import { matchAllRoutes } from '../core/routing/index.js';
import { normalizeTheLocale } from '../i18n/index.js';
import { getSortedPreloadedMatches } from '../prerender/routing.js';
import type { DevPipeline } from './pipeline.js';
-import { default404Page, handle404Response, writeSSRResult, writeWebResponse } from './response.js';
+import { handle404Response, writeSSRResult, writeWebResponse } from './response.js';
type AsyncReturnType<T extends (...args: any) => Promise<any>> = T extends (
...args: any
diff --git a/packages/astro/test/fixtures/reroute/src/pages/blog/oops.astro b/packages/astro/test/fixtures/reroute/src/pages/blog/oops.astro
deleted file mode 100644
index df1f1f76a..000000000
--- a/packages/astro/test/fixtures/reroute/src/pages/blog/oops.astro
+++ /dev/null
@@ -1,11 +0,0 @@
----
-return Astro.rewrite("/404")
----
-<html>
-<head>
- <title>Blog hello</title>
-</head>
-<body>
-<h1>Blog hello</h1>
-</body>
-</html>
diff --git a/packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs b/packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs
new file mode 100644
index 000000000..af13ef19b
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs
@@ -0,0 +1,9 @@
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+ experimental: {
+ rewriting: true
+ },
+ site: "https://example.com"
+});
diff --git a/packages/astro/test/fixtures/rewrite-404-invalid/package.json b/packages/astro/test/fixtures/rewrite-404-invalid/package.json
new file mode 100644
index 000000000..994743ef2
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-404-invalid/package.json
@@ -0,0 +1,8 @@
+{
+ "name": "@test/rewrite-404-invalid",
+ "version": "0.0.0",
+ "private": true,
+ "dependencies": {
+ "astro": "workspace:*"
+ }
+}
diff --git a/packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro b/packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro
new file mode 100644
index 000000000..536eb3501
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro
@@ -0,0 +1,3 @@
+---
+return Astro.rewrite("/404")
+---
diff --git a/packages/astro/test/fixtures/rewrite-runtime-error/astro.config.mjs b/packages/astro/test/fixtures/rewrite-runtime-error/astro.config.mjs
new file mode 100644
index 000000000..af13ef19b
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-runtime-error/astro.config.mjs
@@ -0,0 +1,9 @@
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+ experimental: {
+ rewriting: true
+ },
+ site: "https://example.com"
+});
diff --git a/packages/astro/test/fixtures/rewrite-runtime-error/package.json b/packages/astro/test/fixtures/rewrite-runtime-error/package.json
new file mode 100644
index 000000000..af8c42709
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-runtime-error/package.json
@@ -0,0 +1,8 @@
+{
+ "name": "@test/rewrite-runtime-errror",
+ "version": "0.0.0",
+ "private": true,
+ "dependencies": {
+ "astro": "workspace:*"
+ }
+}
diff --git a/packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/from.astro b/packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/from.astro
new file mode 100644
index 000000000..057629c36
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/from.astro
@@ -0,0 +1,4 @@
+---
+return Astro.rewrite('/errors/to');
+---
+<div></div>
diff --git a/packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/to.astro b/packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/to.astro
new file mode 100644
index 000000000..65be84e8b
--- /dev/null
+++ b/packages/astro/test/fixtures/rewrite-runtime-error/src/pages/errors/to.astro
@@ -0,0 +1,4 @@
+---
+throw new Error('Custom error')
+---
+<div></div>
diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js
index 3808a9181..d66badaac 100644
--- a/packages/astro/test/rewrite.test.js
+++ b/packages/astro/test/rewrite.test.js
@@ -55,11 +55,10 @@ describe('Dev reroute', () => {
assert.equal($('h1').text(), 'Index');
});
- it('should render the 404 built-in page', async () => {
- const html = await fixture.fetch('/blog/oops').then((res) => res.text());
- const $ = cheerioLoad(html);
+ it('should return a 404', async () => {
+ const response = await fixture.fetch('/blog/oops');
- assert.equal($('h1').text(), '404: Not found');
+ assert.equal(response.status, 404);
});
});
@@ -120,6 +119,21 @@ describe('Build reroute', () => {
});
});
+describe('SSR route', () => {
+ it("should not build if a user tries to use rewrite('/404') in static pages", async () => {
+ try {
+ const fixture = await loadFixture({
+ root: './fixtures/rewrite-404-invalid/',
+ });
+ await fixture.build();
+ assert.fail('It should fail.');
+ } catch {
+ // it passes
+ assert.equal(true, true);
+ }
+ });
+});
+
describe('SSR reroute', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
@@ -184,8 +198,7 @@ describe('SSR reroute', () => {
it('should render the 404 built-in page', async () => {
const request = new Request('http://example.com/blog/oops');
const response = await app.render(request);
- const html = await response.text();
- assert.equal(html, 'Not found');
+ assert.equal(response.status, 404);
});
it('should pass the POST data from one page to another', async () => {
@@ -246,3 +259,27 @@ describe('Middleware', () => {
assert.match($('h1').text(), /Index/);
});
});
+
+describe('Runtime error', () => {
+ /** @type {import('./test-utils').Fixture} */
+ let fixture;
+ let devServer;
+
+ before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/rewrite-runtime-error/',
+ });
+ devServer = await fixture.startDevServer();
+ });
+
+ after(async () => {
+ await devServer.stop();
+ });
+
+ it('should render the index page when navigating /reroute ', async () => {
+ const html = await fixture.fetch('/errors/from').then((res) => res.text());
+ const $ = cheerioLoad(html);
+
+ assert.equal($('title').text(), 'Error');
+ });
+});
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 765d42d62..02bc89d0d 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -3466,6 +3466,18 @@ importers:
specifier: workspace:*
version: link:../../..
+ packages/astro/test/fixtures/rewrite-404-invalid:
+ dependencies:
+ astro:
+ specifier: workspace:*
+ version: link:../../..
+
+ packages/astro/test/fixtures/rewrite-runtime-error:
+ dependencies:
+ astro:
+ specifier: workspace:*
+ version: link:../../..
+
packages/astro/test/fixtures/root-srcdir-css:
dependencies:
astro: