From 005d5bacd9c4dca5635da0759d5f73427df68e50 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Wed, 14 Sep 2022 15:47:16 -0400 Subject: [PATCH] Allow custom 404 route to handle API route missing methods (#4594) * Properly allow file uploads in the dev server * Allow custom 404 route to handle API route missing methods * Add a changeset * what was i thinking * Pass through the pathname * Move the try/catch out and into handleRequest * await the result of handleRoute --- .changeset/twelve-days-build.md | 5 + packages/astro/src/core/app/index.ts | 7 + packages/astro/src/runtime/server/endpoint.ts | 19 +- .../src/vite-plugin-astro-server/index.ts | 308 +++++++++++------- packages/astro/test/dev-routing.test.js | 4 + .../src/pages/api/route.js | 6 + packages/astro/test/ssr-404-500-pages.test.js | 94 ++++-- 7 files changed, 289 insertions(+), 154 deletions(-) create mode 100644 .changeset/twelve-days-build.md create mode 100644 packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js diff --git a/.changeset/twelve-days-build.md b/.changeset/twelve-days-build.md new file mode 100644 index 000000000..590c54272 --- /dev/null +++ b/.changeset/twelve-days-build.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Allow custom 404 route to handle API route missing methods diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index d8b003b52..8149d5874 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -202,6 +202,13 @@ export class App { }); if (result.type === 'response') { + if(result.response.headers.get('X-Astro-Response') === 'Not-Found') { + const fourOhFourRequest = new Request(new URL('/404', request.url)); + const fourOhFourRouteData = this.match(fourOhFourRequest); + if(fourOhFourRouteData) { + return this.render(fourOhFourRequest, fourOhFourRouteData); + } + } return result.response; } else { const body = result.body; diff --git a/packages/astro/src/runtime/server/endpoint.ts b/packages/astro/src/runtime/server/endpoint.ts index a5ecc29e0..082290c9e 100644 --- a/packages/astro/src/runtime/server/endpoint.ts +++ b/packages/astro/src/runtime/server/endpoint.ts @@ -18,13 +18,24 @@ function getHandlerFromModule(mod: EndpointHandler, method: string) { } /** Renders an endpoint request to completion, returning the body. */ -export async function renderEndpoint(mod: EndpointHandler, request: Request, params: Params) { +export async function renderEndpoint( + mod: EndpointHandler, + request: Request, + params: Params +) { const chosenMethod = request.method?.toLowerCase(); const handler = getHandlerFromModule(mod, chosenMethod); if (!handler || typeof handler !== 'function') { - throw new Error( - `Endpoint handler not found! Expected an exported function for "${chosenMethod}"` - ); + // No handler found, so this should be a 404. Using a custom header + // to signal to the renderer that this is an internal 404 that should + // be handled by a custom 404 route if possible. + let response = new Response(null, { + status: 404, + headers: { + 'X-Astro-Response': 'Not-Found' + }, + }); + return response; } if (handler.length > 1) { diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts index e0ca6f07f..a1448d553 100644 --- a/packages/astro/src/vite-plugin-astro-server/index.ts +++ b/packages/astro/src/vite-plugin-astro-server/index.ts @@ -1,7 +1,7 @@ import type http from 'http'; import mime from 'mime'; import type * as vite from 'vite'; -import type { AstroConfig, ManifestData } from '../@types/astro'; +import type { AstroConfig, ManifestData, SSRManifest } from '../@types/astro'; import type { SSROptions } from '../core/render/dev/index'; import { Readable } from 'stream'; @@ -28,13 +28,8 @@ interface AstroPluginOptions { logging: LogOptions; } -function truncateString(str: string, n: number) { - if (str.length > n) { - return str.substring(0, n) + '…'; - } else { - return str; - } -} +type AsyncReturnType Promise> = + T extends (...args: any) => Promise ? R : any function writeHtmlResponse(res: http.ServerResponse, statusCode: number, html: string) { res.writeHead(statusCode, { @@ -180,6 +175,68 @@ export function baseMiddleware( }; } +async function matchRoute( + pathname: string, + routeCache: RouteCache, + viteServer: vite.ViteDevServer, + logging: LogOptions, + manifest: ManifestData, + config: AstroConfig, +) { + const matches = matchAllRoutes(pathname, manifest); + + for await (const maybeRoute of matches) { + const filePath = new URL(`./${maybeRoute.component}`, config.root); + const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer }); + const [, mod] = preloadedComponent; + // attempt to get static paths + // if this fails, we have a bad URL match! + const paramsAndPropsRes = await getParamsAndProps({ + mod, + route: maybeRoute, + routeCache, + pathname: pathname, + logging, + ssr: config.output === 'server', + }); + + if (paramsAndPropsRes !== GetParamsAndPropsError.NoMatchingStaticPath) { + return { + route: maybeRoute, + filePath, + preloadedComponent, + mod, + }; + } + } + + if (matches.length) { + warn( + logging, + 'getStaticPaths', + `Route pattern matched, but no matching static path found. (${pathname})` + ); + } + + log404(logging, pathname); + const custom404 = getCustom404Route(config, manifest); + + if (custom404) { + const filePath = new URL(`./${custom404.component}`, config.root); + const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer }); + const [, mod] = preloadedComponent; + + return { + route: custom404, + filePath, + preloadedComponent, + mod, + }; + } + + return undefined; +} + /** The main logic to route dev server requests to pages in Astro. */ async function handleRequest( routeCache: RouteCache, @@ -190,7 +247,6 @@ async function handleRequest( req: http.IncomingMessage, res: http.ServerResponse ) { - const reqStart = performance.now(); const origin = `${viteServer.config.server.https ? 'https' : 'http'}://${req.headers.host}`; const buildingToSSR = config.output === 'server'; // Ignore `.html` extensions and `index.html` in request URLS to ensure that @@ -217,7 +273,7 @@ async function handleRequest( if (!(req.method === 'GET' || req.method === 'HEAD')) { let bytes: Uint8Array[] = []; await new Promise((resolve) => { - req.on('data', (part) => { + req.on('data', part => { bytes.push(part); }); req.on('end', resolve); @@ -225,6 +281,62 @@ async function handleRequest( body = Buffer.concat(bytes); } + let filePath: URL | undefined; + try { + const matchedRoute = await matchRoute( + pathname, + routeCache, + viteServer, + logging, + manifest, + config + ); + filePath = matchedRoute?.filePath; + + return await handleRoute( + matchedRoute, + url, + pathname, + body, + origin, + routeCache, + viteServer, + manifest, + logging, + config, + req, + res + ); + } catch(_err) { + const err = fixViteErrorMessage(_err, viteServer, filePath); + const errorWithMetadata = collectErrorMetadata(err); + error(logging, null, msg.formatErrorMessage(errorWithMetadata)); + handle500Response(viteServer, origin, req, res, errorWithMetadata); + } +} + +async function handleRoute( + matchedRoute: AsyncReturnType, + url: URL, + pathname: string, + body: ArrayBuffer | undefined, + origin: string, + routeCache: RouteCache, + viteServer: vite.ViteDevServer, + manifest: ManifestData, + logging: LogOptions, + config: AstroConfig, + req: http.IncomingMessage, + res: http.ServerResponse +): Promise { + if (!matchedRoute) { + return handle404Response(origin, config, req, res); + } + + const filePath: URL | undefined = matchedRoute.filePath; + const { route, preloadedComponent, mod } = matchedRoute; + const buildingToSSR = config.output === 'server'; + // Headers are only available when using SSR. const request = createRequest({ url, @@ -236,123 +348,75 @@ async function handleRequest( clientAddress: buildingToSSR ? req.socket.remoteAddress : undefined, }); - async function matchRoute() { - const matches = matchAllRoutes(pathname, manifest); + // attempt to get static paths + // if this fails, we have a bad URL match! + const paramsAndPropsRes = await getParamsAndProps({ + mod, + route, + routeCache, + pathname: pathname, + logging, + ssr: config.output === 'server', + }); - for await (const maybeRoute of matches) { - const filePath = new URL(`./${maybeRoute.component}`, config.root); - const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer }); - const [, mod] = preloadedComponent; - // attempt to get static paths - // if this fails, we have a bad URL match! - const paramsAndPropsRes = await getParamsAndProps({ - mod, - route: maybeRoute, - routeCache, - pathname: pathname, - logging, - ssr: config.output === 'server', - }); + const options: SSROptions = { + astroConfig: config, + filePath, + logging, + mode: 'development', + origin, + pathname: pathname, + route, + routeCache, + viteServer, + request, + }; - if (paramsAndPropsRes !== GetParamsAndPropsError.NoMatchingStaticPath) { - return { - route: maybeRoute, - filePath, - preloadedComponent, - mod, - }; - } - } - - if (matches.length) { - warn( - logging, - 'getStaticPaths', - `Route pattern matched, but no matching static path found. (${pathname})` - ); - } - - log404(logging, pathname); - const custom404 = getCustom404Route(config, manifest); - - if (custom404) { - const filePath = new URL(`./${custom404.component}`, config.root); - const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer }); - const [, mod] = preloadedComponent; - - return { - route: custom404, - filePath, - preloadedComponent, - mod, - }; - } - - return undefined; - } - - let filePath: URL | undefined; - try { - const matchedRoute = await matchRoute(); - - if (!matchedRoute) { - return handle404Response(origin, config, req, res); - } - - const { route, preloadedComponent, mod } = matchedRoute; - filePath = matchedRoute.filePath; - - // attempt to get static paths - // if this fails, we have a bad URL match! - const paramsAndPropsRes = await getParamsAndProps({ - mod, - route, - routeCache, - pathname: pathname, - logging, - ssr: config.output === 'server', - }); - - const options: SSROptions = { - astroConfig: config, - filePath, - logging, - mode: 'development', - origin, - pathname: pathname, - route, - routeCache, - viteServer, - request, - }; - - // Route successfully matched! Render it. - if (route.type === 'endpoint') { - const result = await callEndpoint(options); - if (result.type === 'response') { - await writeWebResponse(res, result.response); - } else { - let contentType = 'text/plain'; - // Dynamic routes don’t include `route.pathname`, so synthesise a path for these (e.g. 'src/pages/[slug].svg') - const filepath = - route.pathname || - route.segments.map((segment) => segment.map((p) => p.content).join('')).join('/'); - const computedMimeType = mime.getType(filepath); - if (computedMimeType) { - contentType = computedMimeType; - } - res.writeHead(200, { 'Content-Type': `${contentType};charset=utf-8` }); - res.end(result.body); + // Route successfully matched! Render it. + if (route.type === 'endpoint') { + const result = await callEndpoint(options); + if (result.type === 'response') { + if(result.response.headers.get('X-Astro-Response') === 'Not-Found') { + const fourOhFourRoute = await matchRoute( + '/404', + routeCache, + viteServer, + logging, + manifest, + config + ); + return handleRoute( + fourOhFourRoute, + new URL('/404', url), + '/404', + body, + origin, + routeCache, + viteServer, + manifest, + logging, + config, + req, + res + ); } + await writeWebResponse(res, result.response); } else { - const result = await ssr(preloadedComponent, options); - return await writeSSRResult(result, res); + let contentType = 'text/plain'; + // Dynamic routes don’t include `route.pathname`, so synthesise a path for these (e.g. 'src/pages/[slug].svg') + const filepath = + route.pathname || + route.segments.map((segment) => segment.map((p) => p.content).join('')).join('/'); + const computedMimeType = mime.getType(filepath); + if (computedMimeType) { + contentType = computedMimeType; + } + res.writeHead(200, { 'Content-Type': `${contentType};charset=utf-8` }); + res.end(result.body); } - } catch (_err) { - const err = fixViteErrorMessage(_err, viteServer, filePath); - const errorWithMetadata = collectErrorMetadata(err); - error(logging, null, msg.formatErrorMessage(errorWithMetadata)); - handle500Response(viteServer, origin, req, res, errorWithMetadata); + } else { + const result = await ssr(preloadedComponent, options); + return await writeSSRResult(result, res); } } diff --git a/packages/astro/test/dev-routing.test.js b/packages/astro/test/dev-routing.test.js index 4a892349e..255d8d249 100644 --- a/packages/astro/test/dev-routing.test.js +++ b/packages/astro/test/dev-routing.test.js @@ -285,6 +285,10 @@ describe('Development Routing', () => { devServer = await fixture.startDevServer(); }); + after(async () => { + await devServer.stop(); + }); + it('200 when loading /index.html', async () => { const response = await fixture.fetch('/index.html'); expect(response.status).to.equal(200); diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js b/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js new file mode 100644 index 000000000..c44461be9 --- /dev/null +++ b/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js @@ -0,0 +1,6 @@ + +export function post() { + return { + body: JSON.stringify({ ok: true }) + }; +} diff --git a/packages/astro/test/ssr-404-500-pages.test.js b/packages/astro/test/ssr-404-500-pages.test.js index 1cfc0098c..2d56ef2f8 100644 --- a/packages/astro/test/ssr-404-500-pages.test.js +++ b/packages/astro/test/ssr-404-500-pages.test.js @@ -13,37 +13,75 @@ describe('404 and 500 pages', () => { output: 'server', adapter: testAdapter(), }); - await fixture.build({}); }); - it('404 page returned when a route does not match', async () => { - const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/some/fake/route'); - const response = await app.render(request); - expect(response.status).to.equal(404); - const html = await response.text(); - const $ = cheerio.load(html); - expect($('h1').text()).to.equal('Something went horribly wrong!'); - }); + describe('Development', () => { + /** @type {import('./test-utils').DevServer} */ + let devServer; + before(async () => { + devServer = await fixture.startDevServer(); + }); - it('404 page returned when a route does not match and passing routeData', async () => { - const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/some/fake/route'); - const routeData = app.match(request, { matchNotFound: true }); - const response = await app.render(request, routeData); - expect(response.status).to.equal(404); - const html = await response.text(); - const $ = cheerio.load(html); - expect($('h1').text()).to.equal('Something went horribly wrong!'); - }); + after(async () => { + await devServer.stop(); + }); - it('500 page returned when there is an error', async () => { - const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/causes-error'); - const response = await app.render(request); - expect(response.status).to.equal(500); - const html = await response.text(); - const $ = cheerio.load(html); - expect($('h1').text()).to.equal('This is an error page'); + it('Returns 404 when hitting an API route with the wrong method', async () => { + let res = await fixture.fetch('/api/route', { + method: 'PUT' + }); + let html = await res.text(); + let $ = cheerio.load(html); + expect($('h1').text()).to.equal(`Something went horribly wrong!`); + }); + }); + + describe('Production', () => { + before(async () => { + await fixture.build({}); + }); + + it('404 page returned when a route does not match', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/some/fake/route'); + const response = await app.render(request); + expect(response.status).to.equal(404); + const html = await response.text(); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('Something went horribly wrong!'); + }); + + it('404 page returned when a route does not match and passing routeData', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/some/fake/route'); + const routeData = app.match(request, { matchNotFound: true }); + const response = await app.render(request, routeData); + expect(response.status).to.equal(404); + const html = await response.text(); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('Something went horribly wrong!'); + }); + + it('500 page returned when there is an error', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/causes-error'); + const response = await app.render(request); + expect(response.status).to.equal(500); + const html = await response.text(); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('This is an error page'); + }); + + it('Returns 404 when hitting an API route with the wrong method', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/api/route', { + method: 'PUT' + }); + const response = await app.render(request); + expect(response.status).to.equal(404); + const html = await response.text(); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal(`Something went horribly wrong!`); + }); }); });