Skip to content

Commit

Permalink
fix(@angular/build): add timeout to route extraction
Browse files Browse the repository at this point in the history
This commit introduces a 30-second timeout for route extraction.

(cherry picked from commit aed726f)
  • Loading branch information
alan-agius4 authored and clydin committed Nov 27, 2024
1 parent 8b83650 commit f26e1b4
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 123 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
31 changes: 5 additions & 26 deletions packages/angular/ssr/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}`,
);
}

/**
Expand Down Expand Up @@ -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;
Expand Down
104 changes: 62 additions & 42 deletions packages/angular/ssr/src/routes/ng-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
);

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];
// 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();
}
2 changes: 1 addition & 1 deletion packages/angular/ssr/src/routes/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
50 changes: 50 additions & 0 deletions packages/angular/ssr/src/utils/promise.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
}
7 changes: 6 additions & 1 deletion packages/angular/ssr/test/app_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading

0 comments on commit f26e1b4

Please sign in to comment.