fix(@angular/build): add timeout to route extraction

This commit introduces a 30-second timeout for route extraction.
This commit is contained in:
Alan Agius 2024-11-27 09:45:19 +00:00 committed by Charles
parent 32d2e2d029
commit aed726fca3
8 changed files with 218 additions and 123 deletions

View File

@ -35,12 +35,12 @@ async function extractRoutes(): Promise<RoutersExtractorWorkerResult> {
const { ɵextractRoutesAndCreateRouteTree: extractRoutesAndCreateRouteTree } =
await loadEsmModuleFromMemory('./main.server.mjs');
const { routeTree, appShellRoute, errors } = await extractRoutesAndCreateRouteTree(
serverURL,
undefined /** manifest */,
outputMode !== undefined /** invokeGetPrerenderParams */,
outputMode === OutputMode.Server /** includePrerenderFallbackRoutes */,
);
const { routeTree, appShellRoute, errors } = await extractRoutesAndCreateRouteTree({
url: serverURL,
invokeGetPrerenderParams: outputMode !== undefined,
includePrerenderFallbackRoutes: outputMode === OutputMode.Server,
signal: AbortSignal.timeout(30_000),
});
return {
errors,

View File

@ -24,6 +24,7 @@ import { sha256 } from './utils/crypto';
import { InlineCriticalCssProcessor } from './utils/inline-critical-css';
import { LRUCache } from './utils/lru-cache';
import { AngularBootstrap, renderAngular } from './utils/ng';
import { promiseWithAbort } from './utils/promise';
import {
buildPathWithParams,
joinUrlParts,
@ -182,10 +183,11 @@ export class AngularServerApp {
}
}
return Promise.race([
this.waitForRequestAbort(request),
return promiseWithAbort(
this.handleRendering(request, matchedRoute, requestContext),
]);
request.signal,
`Request for: ${request.url}`,
);
}
/**
@ -353,29 +355,6 @@ export class AngularServerApp {
return new Response(html, responseInit);
}
/**
* Returns a promise that rejects if the request is aborted.
*
* @param request - The HTTP request object being monitored for abortion.
* @returns A promise that never resolves and rejects with an `AbortError`
* if the request is aborted.
*/
private waitForRequestAbort(request: Request): Promise<never> {
return new Promise<never>((_, reject) => {
request.signal.addEventListener(
'abort',
() => {
const abortError = new Error(
`Request for: ${request.url} was aborted.\n${request.signal.reason}`,
);
abortError.name = 'AbortError';
reject(abortError);
},
{ once: true },
);
});
}
}
let angularServerApp: AngularServerApp | undefined;

View File

@ -14,6 +14,7 @@ import { ServerAssets } from '../assets';
import { Console } from '../console';
import { AngularAppManifest, getAngularAppManifest } from '../manifest';
import { AngularBootstrap, isNgModule } from '../utils/ng';
import { promiseWithAbort } from '../utils/promise';
import { addTrailingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
import {
PrerenderFallback,
@ -521,60 +522,79 @@ export async function getRoutesFromAngularRouterConfig(
* Asynchronously extracts routes from the Angular application configuration
* and creates a `RouteTree` to manage server-side routing.
*
* @param url - The URL for server-side rendering. The URL is used to configure `ServerPlatformLocation`. This configuration is crucial
* for ensuring that API requests for relative paths succeed, which is essential for accurate route extraction.
* See:
* - https://github.com/angular/angular/blob/d608b857c689d17a7ffa33bbb510301014d24a17/packages/platform-server/src/location.ts#L51
* - https://github.com/angular/angular/blob/6882cc7d9eed26d3caeedca027452367ba25f2b9/packages/platform-server/src/http.ts#L44
* @param manifest - An optional `AngularAppManifest` that contains the application's routing and configuration details.
* If not provided, the default manifest is retrieved using `getAngularAppManifest()`.
* @param invokeGetPrerenderParams - A boolean flag indicating whether to invoke `getPrerenderParams` for parameterized SSG routes
* to handle prerendering paths. Defaults to `false`.
* @param includePrerenderFallbackRoutes - A flag indicating whether to include fallback routes in the result. Defaults to `true`.
* @param options - An object containing the following options:
* - `url`: The URL for server-side rendering. The URL is used to configure `ServerPlatformLocation`. This configuration is crucial
* for ensuring that API requests for relative paths succeed, which is essential for accurate route extraction.
* See:
* - https://github.com/angular/angular/blob/d608b857c689d17a7ffa33bbb510301014d24a17/packages/platform-server/src/location.ts#L51
* - https://github.com/angular/angular/blob/6882cc7d9eed26d3caeedca027452367ba25f2b9/packages/platform-server/src/http.ts#L44
* - `manifest`: An optional `AngularAppManifest` that contains the application's routing and configuration details.
* If not provided, the default manifest is retrieved using `getAngularAppManifest()`.
* - `invokeGetPrerenderParams`: A boolean flag indicating whether to invoke `getPrerenderParams` for parameterized SSG routes
* to handle prerendering paths. Defaults to `false`.
* - `includePrerenderFallbackRoutes`: A flag indicating whether to include fallback routes in the result. Defaults to `true`.
* - `signal`: An optional `AbortSignal` that can be used to abort the operation.
*
* @returns A promise that resolves to an object containing:
* - `routeTree`: A populated `RouteTree` containing all extracted routes from the Angular application.
* - `appShellRoute`: The specified route for the app-shell, if configured.
* - `errors`: An array of strings representing any errors encountered during the route extraction process.
*/
export async function extractRoutesAndCreateRouteTree(
url: URL,
manifest: AngularAppManifest = getAngularAppManifest(),
invokeGetPrerenderParams = false,
includePrerenderFallbackRoutes = true,
): Promise<{ routeTree: RouteTree; appShellRoute?: string; errors: string[] }> {
const routeTree = new RouteTree();
const document = await new ServerAssets(manifest).getIndexServerHtml().text();
const bootstrap = await manifest.bootstrap();
const { baseHref, appShellRoute, routes, errors } = await getRoutesFromAngularRouterConfig(
bootstrap,
document,
export function extractRoutesAndCreateRouteTree(options: {
url: URL;
manifest?: AngularAppManifest;
invokeGetPrerenderParams?: boolean;
includePrerenderFallbackRoutes?: boolean;
signal?: AbortSignal;
}): Promise<{ routeTree: RouteTree; appShellRoute?: string; errors: string[] }> {
const {
url,
invokeGetPrerenderParams,
includePrerenderFallbackRoutes,
);
manifest = getAngularAppManifest(),
invokeGetPrerenderParams = false,
includePrerenderFallbackRoutes = true,
signal,
} = options;
for (const { route, ...metadata } of routes) {
if (metadata.redirectTo !== undefined) {
metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo);
}
async function extract(): Promise<{
appShellRoute: string | undefined;
routeTree: RouteTree<{}>;
errors: string[];
}> {
const routeTree = new RouteTree();
const document = await new ServerAssets(manifest).getIndexServerHtml().text();
const bootstrap = await manifest.bootstrap();
const { baseHref, appShellRoute, routes, errors } = await getRoutesFromAngularRouterConfig(
bootstrap,
document,
url,
invokeGetPrerenderParams,
includePrerenderFallbackRoutes,
);
// Remove undefined fields
// Helps avoid unnecessary test updates
for (const [key, value] of Object.entries(metadata)) {
if (value === undefined) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (metadata as any)[key];
for (const { route, ...metadata } of routes) {
if (metadata.redirectTo !== undefined) {
metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo);
}
// Remove undefined fields
// Helps avoid unnecessary test updates
for (const [key, value] of Object.entries(metadata)) {
if (value === undefined) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (metadata as any)[key];
}
}
const fullRoute = joinUrlParts(baseHref, route);
routeTree.insert(fullRoute, metadata);
}
const fullRoute = joinUrlParts(baseHref, route);
routeTree.insert(fullRoute, metadata);
return {
appShellRoute,
routeTree,
errors,
};
}
return {
appShellRoute,
routeTree,
errors,
};
return signal ? promiseWithAbort(extract(), signal, 'Routes extraction') : extract();
}

View File

@ -54,7 +54,7 @@ export class ServerRouter {
// Create and store a new promise for the build process.
// This prevents concurrent builds by re-using the same promise.
ServerRouter.#extractionPromise ??= extractRoutesAndCreateRouteTree(url, manifest)
ServerRouter.#extractionPromise ??= extractRoutesAndCreateRouteTree({ url, manifest })
.then(({ routeTree, errors }) => {
if (errors.length > 0) {
throw new Error(

View File

@ -0,0 +1,50 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/
/**
* Creates a promise that resolves with the result of the provided `promise` or rejects with an
* `AbortError` if the `AbortSignal` is triggered before the promise resolves.
*
* @param promise - The promise to monitor for completion.
* @param signal - An `AbortSignal` used to monitor for an abort event. If the signal is aborted,
* the returned promise will reject.
* @param errorMessagePrefix - A custom message prefix to include in the error message when the operation is aborted.
* @returns A promise that either resolves with the value of the provided `promise` or rejects with
* an `AbortError` if the `AbortSignal` is triggered.
*
* @throws {AbortError} If the `AbortSignal` is triggered before the `promise` resolves.
*/
export function promiseWithAbort<T>(
promise: Promise<T>,
signal: AbortSignal,
errorMessagePrefix: string,
): Promise<T> {
return new Promise<T>((resolve, reject) => {
const abortHandler = () => {
reject(
new DOMException(`${errorMessagePrefix} was aborted.\n${signal.reason}`, 'AbortError'),
);
};
// Check for abort signal
if (signal.aborted) {
abortHandler();
return;
}
signal.addEventListener('abort', abortHandler, { once: true });
promise
.then(resolve)
.catch(reject)
.finally(() => {
signal.removeEventListener('abort', abortHandler);
});
});
}

View File

@ -139,7 +139,12 @@ describe('AngularServerApp', () => {
controller.abort();
});
await expectAsync(app.handle(request)).toBeRejectedWithError(/Request for: .+ was aborted/);
try {
await app.handle(request);
throw new Error('Should not be called.');
} catch (e) {
expect(e).toBeInstanceOf(DOMException);
}
});
it('should return configured headers for pages with specific header settings', async () => {

View File

@ -41,7 +41,7 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url });
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/', renderMode: RenderMode.Server },
@ -60,7 +60,7 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { errors } = await extractRoutesAndCreateRouteTree(url);
const { errors } = await extractRoutesAndCreateRouteTree({ url });
expect(errors[0]).toContain(
`Invalid '/invalid' route configuration: the path cannot start with a slash.`,
);
@ -85,7 +85,10 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, true);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
invokeGetPrerenderParams: true,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/user/joe/role/admin', renderMode: RenderMode.Prerender },
@ -119,7 +122,10 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, true);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
invokeGetPrerenderParams: true,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/home', renderMode: RenderMode.Server },
@ -154,7 +160,10 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, true);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
invokeGetPrerenderParams: true,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/home', renderMode: RenderMode.Server },
@ -201,12 +210,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ true,
/** includePrerenderFallbackRoutes */ true,
);
invokeGetPrerenderParams: true,
includePrerenderFallbackRoutes: true,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/', renderMode: RenderMode.Prerender, redirectTo: '/some' },
@ -244,7 +252,7 @@ describe('extractRoutesAndCreateRouteTree', () => {
[{ path: '**', renderMode: RenderMode.Server }],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url });
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/', renderMode: RenderMode.Server, redirectTo: '/some' },
@ -274,7 +282,10 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, false);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
invokeGetPrerenderParams: false,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
{ route: '/home', renderMode: RenderMode.Server },
@ -304,12 +315,12 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ true,
/** includePrerenderFallbackRoutes */ false,
);
invokeGetPrerenderParams: true,
includePrerenderFallbackRoutes: false,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
@ -344,12 +355,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ true,
/** includePrerenderFallbackRoutes */ true,
);
invokeGetPrerenderParams: true,
includePrerenderFallbackRoutes: true,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
@ -372,12 +382,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { errors } = await extractRoutesAndCreateRouteTree(
const { errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);
invokeGetPrerenderParams: false,
includePrerenderFallbackRoutes: false,
});
expect(errors).toHaveSize(0);
});
@ -391,12 +400,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
],
);
const { errors } = await extractRoutesAndCreateRouteTree(
const { errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);
invokeGetPrerenderParams: false,
includePrerenderFallbackRoutes: false,
});
expect(errors).toHaveSize(1);
expect(errors[0]).toContain(
@ -413,12 +421,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
[{ path: 'home', renderMode: RenderMode.Server }],
);
const { errors } = await extractRoutesAndCreateRouteTree(
const { errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);
invokeGetPrerenderParams: false,
includePrerenderFallbackRoutes: false,
});
expect(errors).toHaveSize(1);
expect(errors[0]).toContain(
@ -429,12 +436,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
it('should use wildcard configuration when no Angular routes are defined', async () => {
setAngularAppTestingManifest([], [{ path: '**', renderMode: RenderMode.Server, status: 201 }]);
const { errors, routeTree } = await extractRoutesAndCreateRouteTree(
const { errors, routeTree } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);
invokeGetPrerenderParams: false,
includePrerenderFallbackRoutes: false,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([
@ -449,12 +455,11 @@ describe('extractRoutesAndCreateRouteTree', () => {
/** baseHref*/ './example',
);
const { routeTree, errors } = await extractRoutesAndCreateRouteTree(
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ true,
/** includePrerenderFallbackRoutes */ true,
);
invokeGetPrerenderParams: true,
includePrerenderFallbackRoutes: true,
});
expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([

View File

@ -0,0 +1,36 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/
import { setTimeout } from 'node:timers/promises';
import { promiseWithAbort } from '../../src/utils/promise';
describe('promiseWithAbort', () => {
it('should reject with an AbortError when the signal is aborted', async () => {
const abortController = new AbortController();
const promise = promiseWithAbort(setTimeout(500), abortController.signal, 'Test operation');
queueMicrotask(() => {
abortController.abort('Test reason');
});
await expectAsync(promise).toBeRejectedWithError();
});
it('should not reject if the signal is not aborted', async () => {
const promise = promiseWithAbort(
setTimeout(100),
AbortSignal.timeout(10_000),
'Test operation',
);
// Wait briefly to ensure no rejection occurs
await setTimeout(20);
await expectAsync(promise).toBeResolved();
});
});