Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rewrite): correctly update the status code during a rewrite #11352

Merged
merged 4 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/curvy-kids-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes an issue where the rewrites didn't update the status code when using manual i18n routing.
94 changes: 35 additions & 59 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export class RenderContext {
this.routeData = routeData;
componentInstance = component;
this.isRewriting = true;
this.status = 200;
} else {
this.pipeline.logger.error(
'router',
Expand Down Expand Up @@ -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.'
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends (...args: any) => Promise<any>> = T extends (
...args: any
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/rewrite-i18n-manual-routing",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const onRequest = async (_ctx, next) => {
const response = await next('/');
console.log('expected response.status is', response.status);
return response;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<html lang="en">
<body><h1>Expected http status of index page is 200</h1></body>
</html>
27 changes: 27 additions & 0 deletions packages/astro/test/rewrite.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading