Skip to content

Commit

Permalink
Fix 404 status code in dev server (#7711)
Browse files Browse the repository at this point in the history
* chore: update tests

* chore: update tests

* fix(#7516): set response status to 404 when rendering 404 page

* chore: add changeset

* chore: update dev container test

* refactor: improve status handling logic

* chore: remove unused import
  • Loading branch information
natemoo-re authored Jul 18, 2023
1 parent d401866 commit 72bbfac
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-carrots-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix `status` code for custom `404` and `500` pages in the dev server
20 changes: 18 additions & 2 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type AsyncReturnType<T extends (...args: any) => Promise<any>> = T extends (
? R
: any;

interface MatchedRoute {
export interface MatchedRoute {
route: RouteData;
filePath: URL;
resolvedPathname: string;
Expand Down Expand Up @@ -125,12 +125,14 @@ type HandleRoute = {
incomingRequest: http.IncomingMessage;
incomingResponse: http.ServerResponse;
manifest: SSRManifest;
status?: number;
};

export async function handleRoute({
matchedRoute,
url,
pathname,
status = getStatus(matchedRoute),
body,
origin,
env,
Expand Down Expand Up @@ -198,6 +200,7 @@ export async function handleRoute({
matchedRoute: fourOhFourRoute,
url: new URL('/404', url),
pathname: '/404',
status: 404,
body,
origin,
env,
Expand Down Expand Up @@ -236,6 +239,7 @@ export async function handleRoute({
...options,
matchedRoute: fourOhFourRoute,
url: new URL(pathname, url),
status: 404,
body,
origin,
env,
Expand All @@ -246,6 +250,18 @@ export async function handleRoute({
});
}
throwIfRedirectNotAllowed(result, config);
return await writeSSRResult(request, result, incomingResponse);

let response = result;
// Response.status is read-only, so a clone is required to override
if (status && response.status !== status) {
response = new Response(result.body, { ...result, status });
}
return await writeSSRResult(request, response, incomingResponse);
}
}

function getStatus(matchedRoute?: MatchedRoute): number | undefined {
if (!matchedRoute) return 404;
if (matchedRoute.route.route === '/404') return 404;
if (matchedRoute.route.route === '/500') return 500;
}
5 changes: 4 additions & 1 deletion packages/astro/test/custom-404-html.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ describe('Custom 404.html', () => {
});

it('renders 404 for /a', async () => {
const html = await fixture.fetch('/a').then((res) => res.text());
const res = await fixture.fetch('/a');
expect(res.status).to.equal(404);

const html = await res.text();
$ = cheerio.load(html);

expect($('h1').text()).to.equal('Page not found');
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/test/custom-404-injected.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ describe('Custom 404 with injectRoute', () => {
});

it('renders 404 for /a', async () => {
const html = await fixture.fetch('/a').then((res) => res.text());
const res = await fixture.fetch('/a');
expect(res.status).to.equal(404);

const html = await res.text();
$ = cheerio.load(html);

expect($('h1').text()).to.equal('Page not found');
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/test/custom-404-md.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ describe('Custom 404 Markdown', () => {
});

it('renders 404 for /abc', async () => {
const html = await fixture.fetch('/a').then((res) => res.text());
const res = await fixture.fetch('/a');
expect(res.status).to.equal(404);

const html = await res.text();
$ = cheerio.load(html);

expect($('h1').text()).to.equal('Page not found');
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/test/custom-404-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ describe('Custom 404 server', () => {
});

it('renders 404 for /a', async () => {
const html = await fixture.fetch('/a').then((res) => res.text());
const res = await fixture.fetch('/a');
expect(res.status).to.equal(404);

const html = await res.text();
$ = cheerio.load(html);

expect($('h1').text()).to.equal('Page not found');
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/test/custom-404.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ describe('Custom 404', () => {
});

it('renders 404 for /a', async () => {
const html = await fixture.fetch('/a').then((res) => res.text());
const res = await fixture.fetch('/a');
expect(res.status).to.equal(404);

const html = await res.text();
$ = cheerio.load(html);

expect($('h1').text()).to.equal('Page not found');
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/units/dev/dev.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ describe('dev container', () => {
await r.done;
const doc = await r.text();
expect(doc).to.match(/<h1>Custom 404<\/h1>/);
expect(r.res.statusCode).to.equal(200);
expect(r.res.statusCode).to.equal(404);
}
{
// A non-existent page also serves the custom 404 page.
Expand All @@ -214,7 +214,7 @@ describe('dev container', () => {
await r.done;
const doc = await r.text();
expect(doc).to.match(/<h1>Custom 404<\/h1>/);
expect(r.res.statusCode).to.equal(200);
expect(r.res.statusCode).to.equal(404);
}
}
);
Expand Down

0 comments on commit 72bbfac

Please sign in to comment.