fix(ssr): correctly call the middleware when rendering error pages (#8200)

This commit is contained in:
Emanuele Stoppa 2023-08-25 12:23:58 +01:00 committed by GitHub
parent 660e6f80f4
commit 46d0a0b006
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 81 additions and 26 deletions

View file

@ -9,7 +9,7 @@ import type {
import type { SinglePageBuiltModule } from '../build/types'; import type { SinglePageBuiltModule } from '../build/types';
import { getSetCookiesFromResponse } from '../cookies/index.js'; import { getSetCookiesFromResponse } from '../cookies/index.js';
import { consoleLogDestination } from '../logger/console.js'; import { consoleLogDestination } from '../logger/console.js';
import { error, type LogOptions } from '../logger/core.js'; import { error, type LogOptions, warn } from '../logger/core.js';
import { import {
collapseDuplicateSlashes, collapseDuplicateSlashes,
prependForwardSlash, prependForwardSlash,
@ -56,6 +56,8 @@ export class App {
}; };
#baseWithoutTrailingSlash: string; #baseWithoutTrailingSlash: string;
#pipeline: SSRRoutePipeline; #pipeline: SSRRoutePipeline;
#onRequest: MiddlewareEndpointHandler | undefined;
#middlewareLoaded: boolean;
constructor(manifest: SSRManifest, streaming = true) { constructor(manifest: SSRManifest, streaming = true) {
this.#manifest = manifest; this.#manifest = manifest;
@ -65,6 +67,7 @@ export class App {
this.#routeDataToRouteInfo = new Map(manifest.routes.map((route) => [route.routeData, route])); this.#routeDataToRouteInfo = new Map(manifest.routes.map((route) => [route.routeData, route]));
this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#manifest.base); this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#manifest.base);
this.#pipeline = new SSRRoutePipeline(this.#createEnvironment(streaming)); this.#pipeline = new SSRRoutePipeline(this.#createEnvironment(streaming));
this.#middlewareLoaded = false;
} }
set setManifest(newManifest: SSRManifest) { set setManifest(newManifest: SSRManifest) {
@ -128,7 +131,21 @@ export class App {
if (!routeData || routeData.prerender) return undefined; if (!routeData || routeData.prerender) return undefined;
return routeData; return routeData;
} }
async #getOnRequest() {
if (this.#manifest.middlewareEntryPoint && !this.#middlewareLoaded) {
try {
const middleware = await import(this.#manifest.middlewareEntryPoint);
this.#pipeline.setMiddlewareFunction(middleware.onRequest as MiddlewareEndpointHandler);
} catch (e) {
warn(this.#logging, 'SSR', "Couldn't load the middleware entry point");
}
}
this.#middlewareLoaded = true;
}
async render(request: Request, routeData?: RouteData, locals?: object): Promise<Response> { async render(request: Request, routeData?: RouteData, locals?: object): Promise<Response> {
await this.#getOnRequest();
// Handle requests with duplicate slashes gracefully by cloning with a cleaned-up request URL // Handle requests with duplicate slashes gracefully by cloning with a cleaned-up request URL
if (request.url !== collapseDuplicateSlashes(request.url)) { if (request.url !== collapseDuplicateSlashes(request.url)) {
request = new Request(collapseDuplicateSlashes(request.url), request); request = new Request(collapseDuplicateSlashes(request.url), request);
@ -156,10 +173,6 @@ export class App {
); );
let response; let response;
try { try {
// NOTE: ideally we could set the middleware function just once, but we don't have the infrastructure to that yet
if (mod.onRequest) {
this.#pipeline.setMiddlewareFunction(mod.onRequest as MiddlewareEndpointHandler);
}
response = await this.#pipeline.renderRoute(renderContext, pageModule); response = await this.#pipeline.renderRoute(renderContext, pageModule);
} catch (err: any) { } catch (err: any) {
if (err instanceof EndpointNotFoundError) { if (err instanceof EndpointNotFoundError) {

View file

@ -49,6 +49,7 @@ export type SSRManifest = {
componentMetadata: SSRResult['componentMetadata']; componentMetadata: SSRResult['componentMetadata'];
pageModule?: SinglePageBuiltModule; pageModule?: SinglePageBuiltModule;
pageMap?: Map<ComponentPath, ImportComponentInstance>; pageMap?: Map<ComponentPath, ImportComponentInstance>;
middlewareEntryPoint: string | undefined;
}; };
export type SerializedSSRManifest = Omit< export type SerializedSSRManifest = Omit<

View file

@ -77,6 +77,13 @@ export class BuildPipeline extends Pipeline {
return this.#manifest; return this.#manifest;
} }
async retrieveMiddlewareFunction() {
if (this.#internals.middlewareEntryPoint) {
const middleware = await import(this.#internals.middlewareEntryPoint.toString());
this.setMiddlewareFunction(middleware.onRequest);
}
}
/** /**
* The SSR build emits two important files: * The SSR build emits two important files:
* - dist/server/manifest.mjs * - dist/server/manifest.mjs

View file

@ -10,7 +10,6 @@ import type {
ComponentInstance, ComponentInstance,
GetStaticPathsItem, GetStaticPathsItem,
ImageTransform, ImageTransform,
MiddlewareEndpointHandler,
RouteData, RouteData,
RouteType, RouteType,
SSRError, SSRError,
@ -138,6 +137,7 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn
); );
} }
const buildPipeline = new BuildPipeline(opts, internals, manifest); const buildPipeline = new BuildPipeline(opts, internals, manifest);
await buildPipeline.retrieveMiddlewareFunction();
const outFolder = ssr const outFolder = ssr
? opts.settings.config.build.server ? opts.settings.config.build.server
: getOutDirWithinCwd(opts.settings.config.outDir); : getOutDirWithinCwd(opts.settings.config.outDir);
@ -248,10 +248,6 @@ async function generatePage(
.reduce(mergeInlineCss, []); .reduce(mergeInlineCss, []);
const pageModulePromise = ssrEntry.page; const pageModulePromise = ssrEntry.page;
const onRequest = ssrEntry.onRequest;
if (onRequest) {
pipeline.setMiddlewareFunction(onRequest as MiddlewareEndpointHandler);
}
if (!pageModulePromise) { if (!pageModulePromise) {
throw new Error( throw new Error(
@ -612,5 +608,8 @@ export function createBuildManifest(
? new URL(settings.config.base, settings.config.site).toString() ? new URL(settings.config.base, settings.config.site).toString()
: settings.config.site, : settings.config.site,
componentMetadata: internals.componentMetadata, componentMetadata: internals.componentMetadata,
middlewareEntryPoint: internals.middlewareEntryPoint
? internals.middlewareEntryPoint.toString()
: undefined,
}; };
} }

View file

@ -93,14 +93,19 @@ export function pluginManifest(
} }
const manifest = await createManifest(options, internals); const manifest = await createManifest(options, internals);
const shouldPassMiddlewareEntryPoint =
// TODO: remove in Astro 4.0
options.settings.config.build.excludeMiddleware ||
options.settings.adapter?.adapterFeatures?.edgeMiddleware;
await runHookBuildSsr({ await runHookBuildSsr({
config: options.settings.config, config: options.settings.config,
manifest, manifest,
logging: options.logging, logging: options.logging,
entryPoints: internals.entryPoints, entryPoints: internals.entryPoints,
middlewareEntryPoint: internals.middlewareEntryPoint, middlewareEntryPoint: shouldPassMiddlewareEntryPoint
? internals.middlewareEntryPoint
: undefined,
}); });
// TODO: use the manifest entry chunk instead
const code = injectManifest(manifest, internals.manifestEntryChunk); const code = injectManifest(manifest, internals.manifestEntryChunk);
mutate(internals.manifestEntryChunk, 'server', code); mutate(internals.manifestEntryChunk, 'server', code);
}, },
@ -232,6 +237,10 @@ function buildManifest(
// Set this to an empty string so that the runtime knows not to try and load this. // Set this to an empty string so that the runtime knows not to try and load this.
entryModules[BEFORE_HYDRATION_SCRIPT_ID] = ''; entryModules[BEFORE_HYDRATION_SCRIPT_ID] = '';
} }
const isEdgeMiddleware =
// TODO: remove in Astro 4.0
settings.config.build.excludeMiddleware ||
settings.adapter?.adapterFeatures?.edgeMiddleware;
const ssrManifest: SerializedSSRManifest = { const ssrManifest: SerializedSSRManifest = {
adapterName: opts.settings.adapter?.name ?? '', adapterName: opts.settings.adapter?.name ?? '',
@ -245,6 +254,7 @@ function buildManifest(
clientDirectives: Array.from(settings.clientDirectives), clientDirectives: Array.from(settings.clientDirectives),
entryModules, entryModules,
assets: staticFiles.map(prefixAssetPath), assets: staticFiles.map(prefixAssetPath),
middlewareEntryPoint: !isEdgeMiddleware ? internals.middlewareEntryPoint?.toString() : undefined,
}; };
return ssrManifest; return ssrManifest;

View file

@ -4,6 +4,7 @@ import { addRollupInput } from '../add-rollup-input.js';
import type { BuildInternals } from '../internal'; import type { BuildInternals } from '../internal';
import type { AstroBuildPlugin } from '../plugin'; import type { AstroBuildPlugin } from '../plugin';
import type { StaticBuildOptions } from '../types'; import type { StaticBuildOptions } from '../types';
import { getOutputDirectory } from '../../../prerender/utils.js';
export const MIDDLEWARE_MODULE_ID = '@astro-middleware'; export const MIDDLEWARE_MODULE_ID = '@astro-middleware';
@ -56,8 +57,9 @@ export function vitePluginMiddleware(
if (chunk.type === 'asset') { if (chunk.type === 'asset') {
continue; continue;
} }
if (chunk.fileName === 'middleware.mjs' && opts.settings.config.build.excludeMiddleware) { if (chunk.fileName === 'middleware.mjs') {
internals.middlewareEntryPoint = new URL(chunkName, opts.settings.config.build.server); const outputDirectory = getOutputDirectory(opts.settings.config);
internals.middlewareEntryPoint = new URL(chunkName, outputDirectory);
} }
} }
}, },

View file

@ -4,7 +4,6 @@ import type {
AstroSettings, AstroSettings,
ComponentInstance, ComponentInstance,
ManifestData, ManifestData,
MiddlewareHandler,
RouteData, RouteData,
RuntimeMode, RuntimeMode,
SSRLoadedRenderer, SSRLoadedRenderer,
@ -52,7 +51,6 @@ export interface SinglePageBuiltModule {
/** /**
* The `onRequest` hook exported by the middleware * The `onRequest` hook exported by the middleware
*/ */
onRequest?: MiddlewareHandler<unknown>;
renderers: SSRLoadedRenderer[]; renderers: SSRLoadedRenderer[];
} }

View file

@ -12,6 +12,5 @@ export const RedirectComponentInstance: ComponentInstance = {
export const RedirectSinglePageBuiltModule: SinglePageBuiltModule = { export const RedirectSinglePageBuiltModule: SinglePageBuiltModule = {
page: () => Promise.resolve(RedirectComponentInstance), page: () => Promise.resolve(RedirectComponentInstance),
onRequest: (ctx, next) => next(),
renderers: [], renderers: [],
}; };

View file

@ -99,5 +99,6 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
? new URL(settings.config.base, settings.config.site).toString() ? new URL(settings.config.base, settings.config.site).toString()
: settings.config.site, : settings.config.site,
componentMetadata: new Map(), componentMetadata: new Map(),
middlewareEntryPoint: undefined,
}; };
} }

View file

@ -18,18 +18,20 @@ const first = defineMiddleware(async (context, next) => {
return new Response(JSON.stringify(object), { return new Response(JSON.stringify(object), {
headers: response.headers, headers: response.headers,
}); });
} else if(context.url.pathname === '/clone') { } else if (context.url.pathname === '/clone') {
const response = await next(); const response = await next();
const newResponse = response.clone(); const newResponse = response.clone();
const /** @type {string} */ html = await newResponse.text(); const /** @type {string} */ html = await newResponse.text();
const newhtml = html.replace('<h1>testing</h1>', '<h1>it works</h1>'); const newhtml = html.replace('<h1>testing</h1>', '<h1>it works</h1>');
return new Response(newhtml, { status: 200, headers: response.headers }); return new Response(newhtml, { status: 200, headers: response.headers });
} else { } else {
if(context.url.pathname === '/') { if (context.url.pathname === '/') {
context.cookies.set('foo', 'bar'); context.cookies.set('foo', 'bar');
} }
context.locals.name = 'bar'; context.locals = {
name: 'bar',
};
} }
return await next(); return await next();
}); });

View file

@ -0,0 +1,6 @@
---
const name = Astro.locals.name;
---
<title>Error</title>
<p>{name}</p>

View file

@ -211,6 +211,16 @@ describe('Middleware API in PROD mode, SSR', () => {
expect(text.includes('REDACTED')).to.be.true; expect(text.includes('REDACTED')).to.be.true;
}); });
it('should correctly call the middleware function for 404', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/funky-url');
const routeData = app.match(request, { matchNotFound: true });
const response = await app.render(request, routeData);
const text = await response.text();
expect(text.includes('Error')).to.be.true;
expect(text.includes('bar')).to.be.true;
});
it('the integration should receive the path to the middleware', async () => { it('the integration should receive the path to the middleware', async () => {
fixture = await loadFixture({ fixture = await loadFixture({
root: './fixtures/middleware-dev/', root: './fixtures/middleware-dev/',
@ -219,10 +229,8 @@ describe('Middleware API in PROD mode, SSR', () => {
excludeMiddleware: true, excludeMiddleware: true,
}, },
adapter: testAdapter({ adapter: testAdapter({
setEntryPoints(entryPointsOrMiddleware) { setMiddlewareEntryPoint(entryPointsOrMiddleware) {
if (entryPointsOrMiddleware instanceof URL) {
middlewarePath = entryPointsOrMiddleware; middlewarePath = entryPointsOrMiddleware;
}
}, },
}), }),
}); });
@ -237,6 +245,7 @@ describe('Middleware API in PROD mode, SSR', () => {
throw e; throw e;
} }
}); });
}); });
describe('Middleware with tailwind', () => { describe('Middleware with tailwind', () => {

View file

@ -5,7 +5,13 @@ import { viteID } from '../dist/core/util.js';
* @returns {import('../src/@types/astro').AstroIntegration} * @returns {import('../src/@types/astro').AstroIntegration}
*/ */
export default function ( export default function (
{ provideAddress = true, extendAdapter, setEntryPoints = undefined, setRoutes = undefined } = { {
provideAddress = true,
extendAdapter,
setEntryPoints = undefined,
setMiddlewareEntryPoint = undefined,
setRoutes = undefined,
} = {
provideAddress: true, provideAddress: true,
} }
) { ) {
@ -86,7 +92,9 @@ export default function (
'astro:build:ssr': ({ entryPoints, middlewareEntryPoint }) => { 'astro:build:ssr': ({ entryPoints, middlewareEntryPoint }) => {
if (setEntryPoints) { if (setEntryPoints) {
setEntryPoints(entryPoints); setEntryPoints(entryPoints);
setEntryPoints(middlewareEntryPoint); }
if (setMiddlewareEntryPoint) {
setMiddlewareEntryPoint(middlewareEntryPoint);
} }
}, },
'astro:build:done': ({ routes }) => { 'astro:build:done': ({ routes }) => {