diff options
| author |  Nate Moore <natemoo-re@users.noreply.github.com>
| 2023-08-01 09:52:16 -0500 |
|---|---|---|
| committer |  GitHub <noreply@github.com>
| 2023-08-01 09:52:16 -0500 |
| commit | 298dbb89f2963a547370b6e65cafd2650fdb1b27 (patch) | |
| tree | 6650c12dc5985f5d700319b3fc4a3826fbc40070 /packages/integrations/node | |
| parent | 0b8375fe82a15bfff3f517f98de6454adb2779f1 (diff) | |
| download | astro-298dbb89f2963a547370b6e65cafd2650fdb1b27.tar.gz astro-298dbb89f2963a547370b6e65cafd2650fdb1b27.tar.zst astro-298dbb89f2963a547370b6e65cafd2650fdb1b27.zip | |
Refactor 404 and 500 approach (#7754)
* 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
Diffstat (limited to 'packages/integrations/node')
7 files changed, 250 insertions, 18 deletions
diff --git a/packages/integrations/node/src/nodeMiddleware.ts b/packages/integrations/node/src/nodeMiddleware.ts index 4963afc9f..8d31b6806 100644 --- a/packages/integrations/node/src/nodeMiddleware.ts +++ b/packages/integrations/node/src/nodeMiddleware.ts @@ -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) { diff --git a/packages/integrations/node/test/fixtures/node-middleware/src/pages/404.astro b/packages/integrations/node/test/fixtures/node-middleware/src/pages/404.astro index 4684c8665..79f4944bc 100644 --- a/packages/integrations/node/test/fixtures/node-middleware/src/pages/404.astro +++ b/packages/integrations/node/test/fixtures/node-middleware/src/pages/404.astro @@ -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> diff --git a/packages/integrations/node/test/fixtures/prerender-404/package.json b/packages/integrations/node/test/fixtures/prerender-404/package.json new file mode 100644 index 000000000..dfd109c91 --- /dev/null +++ b/packages/integrations/node/test/fixtures/prerender-404/package.json @@ -0,0 +1,9 @@ +{ + "name": "@test/nodejs-prerender-404", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "@astrojs/node": "workspace:*" + } +} diff --git a/packages/integrations/node/test/fixtures/prerender-404/src/pages/404.astro b/packages/integrations/node/test/fixtures/prerender-404/src/pages/404.astro new file mode 100644 index 000000000..230402bbc --- /dev/null +++ b/packages/integrations/node/test/fixtures/prerender-404/src/pages/404.astro @@ -0,0 +1,5 @@ +--- +export const prerender = true; +--- + +Page does not exist diff --git a/packages/integrations/node/test/fixtures/prerender-404/src/pages/static.astro b/packages/integrations/node/test/fixtures/prerender-404/src/pages/static.astro new file mode 100644 index 000000000..af6bad2fb --- /dev/null +++ b/packages/integrations/node/test/fixtures/prerender-404/src/pages/static.astro @@ -0,0 +1,12 @@ +--- +export const prerender = true; +--- + +<html> +<head> + <title>Static Page</title> +</head> + <body> + <h1>Hello world!</h1> + </body> +</html> diff --git a/packages/integrations/node/test/node-middleware.test.js b/packages/integrations/node/test/node-middleware.test.js index a658f93ef..350e48c7a 100644 --- a/packages/integrations/node/test/node-middleware.test.js +++ b/packages/integrations/node/test/node-middleware.test.js @@ -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'); }); }); }); diff --git a/packages/integrations/node/test/prerender-404.test.js b/packages/integrations/node/test/prerender-404.test.js new file mode 100644 index 000000000..df36fa414 --- /dev/null +++ b/packages/integrations/node/test/prerender-404.test.js @@ -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'); + }); + }); +}); |