Skip to content

Commit

Permalink
fix(rewrite): correctly update the status code during a rewrite (#11352)
Browse files Browse the repository at this point in the history
* fix(rewrite): correctly update the status code during a rewrite

* rebase

* remove `.only`

* remove log
  • Loading branch information
ematipico authored Jul 1, 2024
1 parent 93993b7 commit a55ee02
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 61 deletions.
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.

0 comments on commit a55ee02

Please sign in to comment.