Skip to content

Commit 7ce0519

Browse files
committed
fix(@angular/build): fail build and exclude routes when prerendering fails
When a prerendered route failed to render (such as when a component throws during route activation), the render worker returned null content which was silently skipped. Consequently, no HTML file was written, but the build still reported the route in prerender statistics, included it in prerendered-routes.json, and exited with code 0. Now: - The render worker throws an error if content is null ('The content returned was empty.'). - Prerendering records the error so the build fails with a non-zero exit code. - Prerendered routes recorded for manifest and statistics are derived strictly from routes that produced output files. Closes #33965
1 parent bb72145 commit 7ce0519

4 files changed

Lines changed: 130 additions & 23 deletions

File tree

‎packages/angular/build/src/builders/application/execute-post-bundle.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,13 @@ export async function executePostBundleSteps(
157157
'The "index" option is required when using the "ssg" or "appShell" options.',
158158
);
159159

160-
const { output, warnings, errors, serializableRouteTreeNode } = await prerenderPages(
160+
const {
161+
output,
162+
warnings,
163+
errors,
164+
serializableRouteTreeNode,
165+
prerenderedRoutes: generatedPrerenderedRoutes,
166+
} = await prerenderPages(
161167
workspaceRoot,
162168
baseHref,
163169
appShellOptions,
@@ -171,6 +177,7 @@ export async function executePostBundleSteps(
171177

172178
allErrors.push(...errors);
173179
allWarnings.push(...warnings);
180+
Object.assign(prerenderedRoutes, generatedPrerenderedRoutes);
174181

175182
const indexHasBeenPrerendered = output[indexHtmlOptions.output];
176183
for (const [path, { content, appShellRoute }] of Object.entries(output)) {
@@ -195,10 +202,6 @@ export async function executePostBundleSteps(
195202
const serializableRouteTreeNodeForManifest: WritableSerializableRouteTreeNode = [];
196203
for (const metadata of serializableRouteTreeNode) {
197204
serializableRouteTreeNodeForManifest.push(metadata);
198-
199-
if (metadata.renderMode === RouteRenderMode.Prerender && !metadata.route.includes('*')) {
200-
prerenderedRoutes[metadata.route] = { headers: metadata.headers };
201-
}
202205
}
203206

204207
if (outputMode === OutputMode.Server) {

‎packages/angular/build/src/utils/server-rendering/prerender.ts‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import { readFile } from 'node:fs/promises';
1010
import { extname, posix } from 'node:path';
1111
import { NormalizedApplicationBuildOptions } from '../../builders/application/options';
1212
import { OutputMode } from '../../builders/application/schema';
13-
import { BuildOutputAsset } from '../../tools/esbuild/bundler-execution-result';
13+
import {
14+
BuildOutputAsset,
15+
PrerenderedRoutesRecord,
16+
} from '../../tools/esbuild/bundler-execution-result';
1417
import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-files';
1518
import { assertIsError } from '../error';
1619
import { toPosixPath } from '../path';
@@ -65,6 +68,7 @@ export async function prerenderPages(
6568
output: PrerenderOutput;
6669
warnings: string[];
6770
errors: string[];
71+
prerenderedRoutes: PrerenderedRoutesRecord;
6872
serializableRouteTreeNode: SerializableRouteTreeNode;
6973
}> {
7074
const rawOutputFiles: Record<string, string> = {};
@@ -167,6 +171,7 @@ export async function prerenderPages(
167171
errors,
168172
warnings,
169173
output: {},
174+
prerenderedRoutes: {},
170175
serializableRouteTreeNode,
171176
};
172177
}
@@ -200,10 +205,28 @@ export async function prerenderPages(
200205

201206
errors.push(...renderingErrors);
202207

208+
const prerenderedRoutes: PrerenderedRoutesRecord = {};
209+
const baseHrefPathnameWithLeadingSlash = new URL(baseHref, 'http://localhost').pathname;
210+
211+
for (const metadata of serializableRouteTreeNodeForPrerender) {
212+
const routeWithoutBaseHref = addTrailingSlash(metadata.route).startsWith(
213+
baseHrefPathnameWithLeadingSlash,
214+
)
215+
? addLeadingSlash(metadata.route.slice(baseHrefPathnameWithLeadingSlash.length))
216+
: metadata.route;
217+
218+
const outPath = stripLeadingSlash(posix.join(routeWithoutBaseHref, 'index.html'));
219+
220+
if (output[outPath]) {
221+
prerenderedRoutes[metadata.route] = { headers: metadata.headers };
222+
}
223+
}
224+
203225
return {
204226
errors,
205227
warnings,
206228
output,
229+
prerenderedRoutes,
207230
serializableRouteTreeNode,
208231
};
209232
}
@@ -305,20 +328,20 @@ async function renderPages(
305328
const renderBatchPromise: Promise<RenderResult> = renderWorker.run(urls);
306329
const batchResultPromise = renderBatchPromise
307330
.then((results) => {
308-
for (const { url, content, error } of results) {
309-
if (error) {
310-
errors.push(`An error occurred while prerendering route '${url}'.\n\n${error}`);
331+
for (const result of results) {
332+
if ('error' in result) {
333+
errors.push(
334+
`An error occurred while prerendering route '${result.url}'.\n\n${result.error}`,
335+
);
311336
continue;
312337
}
313338

314-
if (content !== null) {
315-
const routeInfo = routeOutPathMap.get(url);
316-
if (routeInfo) {
317-
output[routeInfo.outPath] = {
318-
content,
319-
appShellRoute: routeInfo.isAppShell,
320-
};
321-
}
339+
const routeInfo = routeOutPathMap.get(result.url);
340+
if (routeInfo) {
341+
output[routeInfo.outPath] = {
342+
content: result.content,
343+
appShellRoute: routeInfo.isAppShell,
344+
};
322345
}
323346
}
324347
})

‎packages/angular/build/src/utils/server-rendering/render-worker.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ export interface RenderWorkerData extends ESMInMemoryFileLoaderWorkerData {
2121
hasSsrEntry: boolean;
2222
}
2323

24-
export interface RenderResultItem {
25-
url: string;
26-
content: string | null;
27-
error?: string;
28-
}
24+
export type RenderResultItem =
25+
| {
26+
url: string;
27+
content: string;
28+
}
29+
| {
30+
url: string;
31+
error: string;
32+
};
2933

3034
export type RenderResult = RenderResultItem[];
3135

@@ -74,12 +78,16 @@ async function renderPages(urls: string[]): Promise<RenderResult> {
7478
for (const currentUrl of urls) {
7579
try {
7680
const content = await renderPage(currentUrl, angularServerApp);
81+
82+
if (content === null) {
83+
throw new Error('The content returned was empty.');
84+
}
85+
7786
results.push({ url: currentUrl, content });
7887
} catch (err) {
7988
assertIsError(err);
8089
results.push({
8190
url: currentUrl,
82-
content: null,
8391
error: err.stack ?? err.message ?? err.code ?? `${err}`,
8492
});
8593
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { ng } from '../../../utils/process';
2+
import { getGlobalVariable } from '../../../utils/env';
3+
import { expectFileNotToExist, readFile, rimraf, writeMultipleFiles } from '../../../utils/fs';
4+
import { match } from 'node:assert';
5+
import { expectToFail } from '../../../utils/utils';
6+
import { useSha } from '../../../utils/project';
7+
import { installWorkspacePackages } from '../../../utils/packages';
8+
import assert from 'node:assert';
9+
10+
export default async function () {
11+
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
12+
if (useWebpackBuilder) {
13+
return;
14+
}
15+
16+
// Forcibly remove in case another test doesn't clean itself up.
17+
await rimraf('node_modules/@angular/ssr');
18+
await ng('add', '@angular/ssr', '--skip-confirmation');
19+
await useSha();
20+
await installWorkspacePackages();
21+
22+
await writeMultipleFiles({
23+
'src/app/app.routes.ts': `
24+
import { Routes } from '@angular/router';
25+
import { Component } from '@angular/core';
26+
27+
@Component({
28+
selector: 'app-home',
29+
standalone: true,
30+
template: '<p>home works!</p>',
31+
})
32+
export class HomeRoute {}
33+
34+
@Component({
35+
selector: 'app-second',
36+
standalone: true,
37+
template: '<p>second works!</p>',
38+
})
39+
export class SecondRoute {
40+
constructor() {
41+
throw new Error('render failure');
42+
}
43+
}
44+
45+
export const routes: Routes = [
46+
{ path: '', component: HomeRoute },
47+
{ path: 'second', component: SecondRoute },
48+
];
49+
`,
50+
'src/app/app.routes.server.ts': `
51+
import { RenderMode, ServerRoute } from '@angular/ssr';
52+
53+
export const serverRoutes: ServerRoute[] = [
54+
{ path: 'second', renderMode: RenderMode.Prerender },
55+
{ path: '**', renderMode: RenderMode.Prerender },
56+
];
57+
`,
58+
});
59+
60+
const { message } = await expectToFail(() => ng('build', '--output-mode=server'));
61+
62+
match(message, /An error occurred while prerendering route '\/second'\./);
63+
64+
await expectFileNotToExist('dist/test-project/browser/second/index.html');
65+
66+
// prerendered-routes.json should only contain successfully prerendered routes
67+
try {
68+
const stats = JSON.parse(await readFile('dist/test-project/prerendered-routes.json'));
69+
assert.strictEqual(stats.routes['/second'], undefined);
70+
} catch {
71+
// If prerendered-routes.json was not emitted on failure, that is also valid.
72+
}
73+
}

0 commit comments

Comments
 (0)