Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-03-10 23:01:26 -05:00
Code Issues Activity

Refactor 404 and 500 approach (#7754)

Browse source 
* fix(app): refactor 404 and 500 approach

* chore: refactor logic

* fix: always treat error as page

* test: migrate ssr-prerender-404 to node adapter

* feat: merge original response metadata with error response

* chore: update lockfile

* chore: trigger ci

* chore(lint): fix lint issue

* fix: ensure merged request has proper status

* fix(node): prerender test

* chore: update test label

* fix(node): improve 404 behavior in middleware mode

* fix(vercel): improve 404 behavior

* fix(netlify): improve 404 behavior

* chore: update test labels

* chore: force ci

* chore: fix lint

* fix: avoid infinite loops

* test: fix failing test in Node 18

* chore: remove volta
This commit is contained in:
Nate Moore 2023-08-01 09:52:16 -05:00 • committed by GitHub
parent e3c23888d3
commit 7e934478ee
7 changed files with 250 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
packages/integrations/node/src/nodeMiddleware.ts
View file

@ -5,7 +5,9 @@ import { createOutgoingHttpHeaders } from './createOutgoingHttpHeaders';
import { responseIterator } from './response-iterator';
import type { Options } from './types';
export default function (app: NodeApp, mode: Options['mode']) {
// Disable no-unused-vars to avoid breaking signature change
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export default function (app: NodeApp, _mode: Options['mode']) {
return async function (
req: IncomingMessage,
res: ServerResponse,
@ -13,8 +15,7 @@ export default function (app: NodeApp, mode: Options['mode']) {
locals?: object
) {
try {
const route =
mode === 'standalone' ? app.match(req, { matchNotFound: true }) : app.match(req);
const route = app.match(req);
if (route) {
try {
const response = await app.render(req, route, locals);
@ -29,8 +30,8 @@ export default function (app: NodeApp, mode: Options['mode']) {
} else if (next) {
return next();
} else {
res.writeHead(404);
res.end('Not found');
const response = await app.render(req);
await writeWebResponse(app, res, response);
}
} catch (err: unknown) {
if (!res.headersSent) {

2
packages/integrations/node/test/fixtures/node-middleware/src/pages/404.astro vendored
View file

@ -9,5 +9,5 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>404</title>
</head>
<body><h1>404!!!!!!!!!!</h1></body>
<body>Page does not exist</body>
</html>

9
packages/integrations/node/test/fixtures/prerender-404/package.json vendored Normal file
View file

@ -0,0 +1,9 @@
{
"name": "@test/nodejs-prerender-404",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*",
"@astrojs/node": "workspace:*"
}
}

5
packages/integrations/node/test/fixtures/prerender-404/src/pages/404.astro vendored Normal file
View file

@ -0,0 +1,5 @@
---
export const prerender = true;
---
Page does not exist

12
packages/integrations/node/test/fixtures/prerender-404/src/pages/static.astro vendored Normal file
View file

@ -0,0 +1,12 @@
---
export const prerender = true;
---
<html>
<head>
<title>Static Page</title>
</head>
<body>
<h1>Hello world!</h1>
</body>
</html>

40
packages/integrations/node/test/node-middleware.test.js
View file

@ -3,35 +3,51 @@ import { loadFixture } from './test-utils.js';
import { expect } from 'chai';
import * as cheerio from 'cheerio';
describe('test 404 cant load', () => {
/**
* @typedef {import('../../../astro/test/test-utils').Fixture} Fixture
*/
async function load() {
const mod = await import(`./fixtures/node-middleware/dist/server/entry.mjs?dropcache=${Date.now()}`);
return mod;
}
describe('behavior from middleware', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let server;
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
process.env.PRERENDER = false;
fixture = await loadFixture({
root: './fixtures/node-middleware/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await load();
let res = startServer();
server = res.server;
});
describe('test 404', async () => {
let devPreview;
before(async () => {
devPreview = await fixture.preview();
});
after(async () => {
await devPreview.stop();
});
after(async () => {
await server.stop();
await fixture.clean();
delete process.env.PRERENDER;
})
describe('404', async () => {
it('when mode is standalone', async () => {
const res = await fixture.fetch('/error-page');
const res = await fetch(`http://${server.host}:${server.port}/error-page`);
expect(res.status).to.equal(404);
const html = await res.text();
const $ = cheerio.load(html);
const h1 = $('h1');
expect(h1.text()).to.equal('404!!!!!!!!!!');
const body = $('body');
expect(body.text()).to.equal('Page does not exist');
});
});
});

189
packages/integrations/node/test/prerender-404.test.js Normal file
View file

@ -0,0 +1,189 @@
import nodejs from '../dist/index.js';
import { loadFixture } from './test-utils.js';
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { fetch } from 'undici';
/**
* @typedef {import('../../../astro/test/test-utils').Fixture} Fixture
*/
async function load() {
const mod = await import(`./fixtures/prerender-404/dist/server/entry.mjs?dropcache=${Date.now()}`);
return mod;
}
describe('Prerender 404', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let server;
describe('With base', async () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
process.env.PRERENDER = true;
fixture = await loadFixture({
base: '/some-base',
root: './fixtures/prerender-404/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await load();
let res = startServer();
server = res.server;
});
after(async () => {
await server.stop();
await fixture.clean();
delete process.env.PRERENDER;
});
it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/static`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Hello world!');
});
it('Can handle prerendered 404', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/missing`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(404);
expect($('body').text()).to.equal('Page does not exist');
});
});
describe('Without base', async () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
process.env.PRERENDER = true;
fixture = await loadFixture({
root: './fixtures/prerender-404/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});
after(async () => {
await server.stop();
await fixture.clean();
delete process.env.PRERENDER;
});
it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/static`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Hello world!');
});
it('Can handle prerendered 404', async () => {
const res = await fetch(`http://${server.host}:${server.port}/missing`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(404);
expect($('body').text()).to.equal('Page does not exist');
});
});
});
describe('Hybrid 404', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let server;
describe('With base', async () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
process.env.PRERENDER = false;
fixture = await loadFixture({
base: '/some-base',
root: './fixtures/prerender-404/',
output: 'hybrid',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});
after(async () => {
await server.stop();
await fixture.clean();
delete process.env.PRERENDER;
});
it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/static`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Hello world!');
});
it('Can handle prerendered 404', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/missing`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(404);
expect($('body').text()).to.equal('Page does not exist');
});
});
describe('Without base', async () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
process.env.PRERENDER = false;
fixture = await loadFixture({
root: './fixtures/prerender-404/',
output: 'hybrid',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});
after(async () => {
await server.stop();
await fixture.clean();
delete process.env.PRERENDER;
});
it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/static`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Hello world!');
});
it('Can handle prerendered 404', async () => {
const res = await fetch(`http://${server.host}:${server.port}/missing`);
const html = await res.text();
const $ = cheerio.load(html);
expect(res.status).to.equal(404);
expect($('body').text()).to.equal('Page does not exist');
});
});
});