From a55ee0268e1ca22597e9b5e6d1f24b4f28ad978b Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 1 Jul 2024 14:55:33 +0100 Subject: [PATCH] fix(rewrite): correctly update the status code during a rewrite (#11352) * fix(rewrite): correctly update the status code during a rewrite * rebase * remove `.only` * remove log --- .changeset/curvy-kids-call.md | 5 + packages/astro/src/core/render-context.ts | 94 +++++++------------ .../src/vite-plugin-astro-server/route.ts | 3 +- .../astro.config.mjs | 13 +++ .../rewrite-i18n-manual-routing/package.json | 8 ++ .../src/middleware.js | 5 + .../src/pages/index.astro | 3 + packages/astro/test/rewrite.test.js | 27 ++++++ pnpm-lock.yaml | 6 ++ 9 files changed, 103 insertions(+), 61 deletions(-) create mode 100644 .changeset/curvy-kids-call.md create mode 100644 packages/astro/test/fixtures/rewrite-i18n-manual-routing/astro.config.mjs create mode 100644 packages/astro/test/fixtures/rewrite-i18n-manual-routing/package.json create mode 100644 packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/middleware.js create mode 100644 packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/pages/index.astro diff --git a/.changeset/curvy-kids-call.md b/.changeset/curvy-kids-call.md new file mode 100644 index 000000000000..72dfbb9ee1ea --- /dev/null +++ b/.changeset/curvy-kids-call.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where the rewrites didn't update the status code when using manual i18n routing. diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 93899129d9e5..d35e635e06fa 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -146,6 +146,7 @@ export class RenderContext { this.routeData = routeData; componentInstance = component; this.isRewriting = true; + this.status = 200; } else { this.pipeline.logger.error( 'router', @@ -230,16 +231,9 @@ export class RenderContext { }); } - createActionAPIContext(): ActionAPIContext { - const renderContext = this; - const { cookies, params, pipeline, url } = this; - const generator = `Astro v${ASTRO_VERSION}`; - const redirect = (path: string, status = 302) => - new Response(null, { status, headers: { Location: path } }); - - const rewrite = async (reroutePayload: RewritePayload) => { - pipeline.logger.debug('router', 'Called rewriting to:', reroutePayload); - if (!this.pipeline.manifest.rewritingEnabled) { + async #executeRewrite(reroutePayload: RewritePayload) { + this.pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); + if (!this.pipeline.manifest.rewritingEnabled) { this.pipeline.logger.error( 'router', 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' @@ -253,23 +247,36 @@ export class RenderContext { } ); } - const [routeData, component, newURL] = await pipeline.tryRewrite( - reroutePayload, - this.request, - this.originalRoute - ); - this.routeData = routeData; - if (reroutePayload instanceof Request) { - this.request = reroutePayload; - } else { - this.request = this.#copyRequest(newURL, this.request); - } - this.url = newURL; - this.cookies = new AstroCookies(this.request); - this.params = getParams(routeData, this.url.pathname); - this.isRewriting = true; - this.pathname = this.url.pathname; - return await this.render(component); + const [routeData, component, newURL] = await this.pipeline.tryRewrite( + reroutePayload, + this.request, + this.originalRoute + ); + this.routeData = routeData; + if (reroutePayload instanceof Request) { + this.request = reroutePayload; + } else { + this.request = this.#copyRequest(newURL, this.request); + } + this.url = new URL(this.request.url); + this.cookies = new AstroCookies(this.request); + this.params = getParams(routeData, this.url.pathname); + this.pathname = this.url.pathname; + this.isRewriting = true; + // we found a route and a component, we can change the status code to 200 + this.status = 200; + return await this.render(component); + } + + createActionAPIContext(): ActionAPIContext { + const renderContext = this; + const { cookies, params, pipeline, url } = this; + const generator = `Astro v${ASTRO_VERSION}`; + const redirect = (path: string, status = 302) => + new Response(null, { status, headers: { Location: path } }); + + const rewrite = async (reroutePayload: RewritePayload) => { + return await this.#executeRewrite(reroutePayload); }; return { @@ -447,38 +454,7 @@ export class RenderContext { }; const rewrite = async (reroutePayload: RewritePayload) => { - if (!this.pipeline.manifest.rewritingEnabled) { - this.pipeline.logger.error( - 'router', - 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' - ); - return new Response( - 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', - { - status: 500, - statusText: - 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', - } - ); - } - pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); - const [routeData, component, newURL] = await pipeline.tryRewrite( - reroutePayload, - this.request, - this.originalRoute - ); - this.routeData = routeData; - if (reroutePayload instanceof Request) { - this.request = reroutePayload; - } else { - this.request = this.#copyRequest(newURL, this.request); - } - this.url = new URL(this.request.url); - this.cookies = new AstroCookies(this.request); - this.params = getParams(routeData, this.url.pathname); - this.pathname = this.url.pathname; - this.isRewriting = true; - return await this.render(component); + return await this.#executeRewrite(reroutePayload); }; return { diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 5a9e6140ce6c..64caf04f0940 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -13,10 +13,9 @@ 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 { handle404Response, writeSSRResult, writeWebResponse } from './response.js'; +import { writeSSRResult, writeWebResponse } from './response.js'; type AsyncReturnType Promise> = T extends ( ...args: any diff --git a/packages/astro/test/fixtures/rewrite-i18n-manual-routing/astro.config.mjs b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/astro.config.mjs new file mode 100644 index 000000000000..5d19c8dcc394 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/astro.config.mjs @@ -0,0 +1,13 @@ +import { defineConfig } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({ + experimental: { + rewriting: true + }, + i18n: { + routing: "manual", + locales: ["en", "es"], + defaultLocale: "en" + } +}); diff --git a/packages/astro/test/fixtures/rewrite-i18n-manual-routing/package.json b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/package.json new file mode 100644 index 000000000000..8a05fe343178 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/rewrite-i18n-manual-routing", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/middleware.js b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/middleware.js new file mode 100644 index 000000000000..540f39f2eba1 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/middleware.js @@ -0,0 +1,5 @@ +export const onRequest = async (_ctx, next) => { + const response = await next('/'); + console.log('expected response.status is', response.status); + return response; +}; diff --git a/packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/pages/index.astro b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/pages/index.astro new file mode 100644 index 000000000000..df13b07d91b5 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-i18n-manual-routing/src/pages/index.astro @@ -0,0 +1,3 @@ + +

Expected http status of index page is 200

+ diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js index 9523922e0ea2..d14fb069a7c7 100644 --- a/packages/astro/test/rewrite.test.js +++ b/packages/astro/test/rewrite.test.js @@ -519,3 +519,30 @@ describe('Runtime error in SSR, custom 500', () => { assert.equal($('h1').text(), 'I am the custom 500'); }); }); + +describe('Runtime error in SSR, custom 500', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + let app; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/rewrite-i18n-manual-routing/', + output: 'server', + adapter: testAdapter(), + }); + await fixture.build(); + app = await fixture.loadTestAdapterApp(); + }); + + it('should return a status 200 when rewriting from the middleware to the homepage', async () => { + const request = new Request('http://example.com/foo'); + const response = await app.render(request); + assert.equal(response.status, 200); + const html = await response.text(); + + const $ = cheerioLoad(html); + + assert.equal($('h1').text(), 'Expected http status of index page is 200'); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3b95062daa27..46d1ddd5ba53 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3529,6 +3529,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/rewrite-i18n-manual-routing: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/rewrite-runtime-error: dependencies: astro: