Skip to content

Commit e2b9c7f

Browse files
PolyterativePolyterative
committed
fix(ssr): harden route handling and 404 responses
1 parent 5e8dd63 commit e2b9c7f

10 files changed

Lines changed: 227 additions & 11 deletions

File tree

‎angular.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,6 @@
9191
"server": false
9292
},
9393
"production": {
94-
"outputMode": "static",
95-
"ssr": false,
96-
"server": false,
9794
"fileReplacements": [
9895
{
9996
"replace": "src/environments/environment.ts",

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
"test:functions:image-metadata": "node --test scripts/tests/image-metadata.test.cjs",
3131
"test:functions:public-id-migrations": "node --test scripts/tests/public-id-migrations.test.cjs",
3232
"test:functions:panel-ratio-audit": "node --test scripts/tests/panel-ratio-audit.test.mjs",
33+
"test:ssr": "pnpm build && node scripts/tests/ssr-smoke.mjs",
34+
"test:ssr:smoke": "node scripts/tests/ssr-smoke.mjs",
3335
"test:ci:full": "pnpm test:functions && pnpm test:unit:ci",
3436
"e2e": "pnpm test:e2e",
3537
"pretest:e2e": "pnpm exec playwright install chromium",

‎scripts/tests/ssr-smoke.mjs‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import assert from 'node:assert/strict';
2+
import { existsSync } from 'node:fs';
3+
import { createServer } from 'node:http';
4+
import { resolve } from 'node:path';
5+
import { pathToFileURL } from 'node:url';
6+
7+
const serverBundlePath = resolve('dist/Patcher/server/server.mjs');
8+
const browserIndexPath = resolve('dist/Patcher/browser/index.csr.html');
9+
10+
assert.ok(existsSync(serverBundlePath), 'SSR server bundle is missing; run `pnpm build` first');
11+
assert.ok(existsSync(browserIndexPath), 'SSR browser index.csr.html is missing; production build is not emitting SSR browser HTML');
12+
13+
process.env['NG_ALLOWED_HOSTS'] = [
14+
process.env['NG_ALLOWED_HOSTS'],
15+
'127.0.0.1',
16+
'localhost',
17+
'patcher.xyz',
18+
].filter(Boolean).join(',');
19+
process.env['SEO_CANONICAL_ORIGIN'] ||= 'https://patcher.xyz';
20+
process.env['VERCEL'] ||= '1';
21+
22+
let failure;
23+
process.on('unhandledRejection', recordFailure);
24+
process.on('uncaughtException', recordFailure);
25+
26+
const serverBundleUrl = pathToFileURL(serverBundlePath).href;
27+
const { app } = await import(serverBundleUrl);
28+
assert.equal(typeof app, 'function', 'SSR server bundle must export app() for local smoke tests');
29+
30+
const vercelShim = await import(pathToFileURL(resolve('api/ssr.mjs')).href);
31+
assert.equal(typeof vercelShim.default, 'function', 'Vercel SSR shim must export a request handler');
32+
33+
const expressApp = app();
34+
const httpServer = createServer(expressApp);
35+
36+
const ssrRouteCases = [
37+
// Home
38+
{ path: '/', expectedText: 'Browse modules' },
39+
{ path: '/home', expectedText: 'Browse modules' },
40+
41+
// Main public browser/detail surfaces
42+
{ path: '/modules' },
43+
{ path: '/modules/browser', expectedText: 'Modules' },
44+
{ path: '/modules/details/1' },
45+
{ path: '/modules/add' },
46+
{ path: '/patches' },
47+
{ path: '/patches/browser', expectedText: 'Patches' },
48+
{ path: '/patches/details/1' },
49+
{ path: '/patches/example-public-id' },
50+
{ path: '/racks' },
51+
{ path: '/racks/browser', expectedText: 'Racks' },
52+
{ path: '/racks/details/1' },
53+
{ path: '/racks/example-public-id' },
54+
{ path: '/manufacturers' },
55+
{ path: '/manufacturers/browser', expectedText: 'Manufacturers' },
56+
{ path: '/manufacturers/details/1' },
57+
58+
// Production-disabled collection routes must not become crawlable soft-200s.
59+
{ path: '/collections', expectedText: 'Page not found!', expectedStatus: 404 },
60+
{ path: '/collections/browser', expectedText: 'Page not found!', expectedStatus: 404 },
61+
{ path: '/collections/example-public-id', expectedText: 'Page not found!', expectedStatus: 404 },
62+
{ path: '/collections/manage/1', expectedText: 'Page not found!', expectedStatus: 404 },
63+
{ path: '/collection/1', expectedText: 'Page not found!', expectedStatus: 404 },
64+
65+
// Info pages
66+
{ path: '/info', expectedStatus: 404 },
67+
{ path: '/info/changelog', expectedText: 'Changelog' },
68+
{ path: '/info/insights' },
69+
70+
// Auth and user account surfaces
71+
{ path: '/auth', expectedStatus: 404 },
72+
{ path: '/auth/login', expectedText: 'Who are you again?' },
73+
{ path: '/auth/signup', expectedText: "We haven't been introduced yet" },
74+
{ path: '/auth/reset-password' },
75+
{ path: '/auth/callback' },
76+
{ path: '/auth/complete-profile' },
77+
{ path: '/u/example-user' },
78+
{ path: '/user', expectedStatus: 404 },
79+
{ path: '/user/account' },
80+
{ path: '/user/area' },
81+
82+
// Admin, retired links, and explicit not-found
83+
{ path: '/admin' },
84+
{ path: '/links/retired' },
85+
{ path: '/404', expectedText: 'Page not found!', expectedStatus: 404 },
86+
];
87+
88+
await new Promise((resolveListen, rejectListen) => {
89+
httpServer.once('error', rejectListen);
90+
httpServer.listen(0, '127.0.0.1', resolveListen);
91+
});
92+
93+
try {
94+
const address = httpServer.address();
95+
assert.ok(address && typeof address === 'object', 'SSR smoke server did not expose a listening port');
96+
const baseUrl = `http://127.0.0.1:${ address.port }`;
97+
98+
for (const routeCase of ssrRouteCases) {
99+
await assertServerRenderedPage(
100+
`${ baseUrl }${ routeCase.path }`,
101+
routeCase.expectedText,
102+
routeCase.expectedStatus ?? 200,
103+
);
104+
}
105+
} catch (error) {
106+
failure = error;
107+
} finally {
108+
await new Promise((resolveClose, rejectClose) => {
109+
httpServer.close(error => error ? rejectClose(error) : resolveClose());
110+
});
111+
}
112+
113+
if (failure) {
114+
console.error(failure);
115+
process.exit(1);
116+
}
117+
118+
function recordFailure(error) {
119+
failure ??= error;
120+
}
121+
122+
async function assertServerRenderedPage(url, expectedText, expectedStatus = 200) {
123+
const response = await fetch(url, {
124+
headers: {
125+
host: 'patcher.xyz',
126+
'x-forwarded-host': 'patcher.xyz',
127+
'x-forwarded-proto': 'https',
128+
'user-agent': 'Googlebot/2.1 (+http://www.google.com/bot.html)',
129+
},
130+
});
131+
const html = await response.text();
132+
133+
assert.equal(response.status, expectedStatus, `${ url } should SSR with HTTP ${ expectedStatus }; body: ${ html.slice(0, 500) }`);
134+
assert.match(html, /<app-root[^>]*>/, `${ url } should include the Angular root`);
135+
if (expectedText) {
136+
assert.ok(html.includes(expectedText), `${ url } should contain server-rendered page text "${ expectedText }"`);
137+
}
138+
assert.doesNotMatch(html, /<app-root[^>]*><\/app-root>/, `${ url } should not return the empty SPA shell`);
139+
assert.doesNotMatch(html, /Internal Server Error/i, `${ url } should not return the SSR error response`);
140+
}

‎src/app/app.routes.server.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { RenderMode, ServerRoute } from '@angular/ssr';
22

33
export const serverRoutes: ServerRoute[] = [
4+
{ path: '404', renderMode: RenderMode.Server, status: 404 },
45
// All routes rendered on-demand (on the server per request, never prerendered)
56
// This gives crawlers real HTML without a build-time database snapshot
67
{ path: '**', renderMode: RenderMode.Server },

‎src/app/features/backbone/404/not-found.component.spec.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
import { NotFoundComponent } from './not-found.component';
2+
import {
3+
Meta,
4+
Title
5+
} from '@angular/platform-browser';
26

37
describe('NotFoundComponent', () => {
48
let comp: NotFoundComponent;
@@ -8,7 +12,7 @@ describe('NotFoundComponent', () => {
812
beforeEach(() => {
913
mockMeta = { updateTag: jasmine.createSpy('updateTag') };
1014
mockTitle = { setTitle: jasmine.createSpy('setTitle') };
11-
comp = new NotFoundComponent(mockMeta as any, mockTitle as any);
15+
comp = new NotFoundComponent(mockMeta as unknown as Meta, mockTitle as unknown as Title);
1216
});
1317

1418
it('creates without error', () => {
@@ -43,4 +47,13 @@ describe('NotFoundComponent', () => {
4347
jasmine.objectContaining({ property: 'og:description' })
4448
);
4549
});
50+
51+
it('ngOnInit sets the server response status to 404 when available', () => {
52+
const responseInit: ResponseInit = {};
53+
comp = new NotFoundComponent(mockMeta as unknown as Meta, mockTitle as unknown as Title, responseInit);
54+
55+
comp.ngOnInit();
56+
57+
expect(responseInit.status).toBe(404);
58+
});
4659
});

‎src/app/features/backbone/404/not-found.component.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import {
22
ChangeDetectionStrategy,
33
Component,
4-
OnInit
4+
Inject,
5+
OnInit,
6+
Optional,
7+
RESPONSE_INIT
58
} from '@angular/core';
69
import {
710
Meta,
@@ -22,10 +25,14 @@ export class NotFoundComponent implements OnInit {
2225

2326
constructor(
2427
private readonly meta: Meta,
25-
private readonly title: Title
28+
private readonly title: Title,
29+
@Optional() @Inject(RESPONSE_INIT) private readonly responseInit?: ResponseInit | null
2630
) { }
2731

2832
ngOnInit(): void {
33+
if (this.responseInit) {
34+
this.responseInit.status = 404;
35+
}
2936
this.title.setTitle('404 - Not Found | patcher.xyz');
3037
this.meta.updateTag({name: 'robots', content: 'noindex, nofollow'});
3138
this.meta.updateTag({name: 'description', content: '404 - Not Found'});

‎src/app/features/backbone/login/reset-password/reset-password-page.component.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export class ResetPasswordPageComponent extends SubManager implements OnInit {
8181
const type = params['type'];
8282

8383
// Also check hash fragment for access_token (Supabase's standard flow)
84-
const hash = window.location.hash;
84+
const hash = this.getBrowserLocationHash();
8585
const hashHasToken = hash.includes('access_token=') || hash.includes('access_token%3D');
8686
const hashHasRecovery = hash.includes('type=recovery') || hash.includes('type%3Drecovery');
8787

@@ -179,4 +179,8 @@ export class ResetPasswordPageComponent extends SubManager implements OnInit {
179179
}
180180

181181
protected readonly SharedConstants = SharedConstants;
182+
183+
private getBrowserLocationHash(): string {
184+
return typeof window === 'undefined' ? '' : window.location.hash;
185+
}
182186
}

‎src/app/features/backbone/login/reset-password/user-reset-password-data.service.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ export class UserResetPasswordDataService extends SubManager implements OnDestro
193193
* Check if URL contains recovery parameters
194194
*/
195195
checkForRecoveryInUrl(): boolean {
196-
const hash = window.location.hash;
196+
const hash = this.getBrowserLocationHash();
197197
const isRecovery = hash.includes('type=recovery') || hash.includes('type%3Drecovery');
198198
this.setRecoverySession(isRecovery);
199199
return isRecovery;
@@ -244,4 +244,8 @@ export class UserResetPasswordDataService extends SubManager implements OnDestro
244244
}
245245
super.ngOnDestroy();
246246
}
247+
248+
private getBrowserLocationHash(): string {
249+
return typeof window === 'undefined' ? '' : window.location.hash;
250+
}
247251
}

‎src/server.ts‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
resolveRequestOrigin,
2828
resolveSsrAllowedHosts
2929
} from './ssr-host-config';
30+
import { environment } from './environments/environment';
3031

3132

3233
const serverDistFolder = dirname(fileURLToPath(import.meta.url));
@@ -55,6 +56,7 @@ export function app(): express.Express {
5556
// All other requests → Angular SSR via CommonEngine
5657
server.use((req, res, next) => {
5758
const { protocol, originalUrl, baseUrl, headers } = req;
59+
const statusCode = resolveSsrStatusCode(originalUrl);
5860
const requestOrigin = resolveRequestOrigin({
5961
protocol,
6062
host: headers.host,
@@ -70,7 +72,7 @@ export function app(): express.Express {
7072
publicPath: browserDistFolder,
7173
providers: [{ provide: APP_BASE_HREF, useValue: baseUrl }],
7274
})
73-
.then((html) => res.send(html))
75+
.then((html) => res.status(statusCode).send(html))
7476
.catch(next);
7577
});
7678

@@ -110,4 +112,45 @@ if (isMainModule(import.meta.url)) {
110112
// eslint-disable-next-line @typescript-eslint/no-explicit-any
111113
export const reqHandler = isProd
112114
? createNodeRequestHandler(app())
113-
: ((_req: any, _res: any, next: any) => next?.());
115+
: ((_req: any, _res: any, next: any) => next?.());
116+
117+
function resolveSsrStatusCode(originalUrl: string): number {
118+
try {
119+
const pathname = new URL(originalUrl, 'https://patcher.xyz').pathname;
120+
return isKnownApplicationRoute(pathname) && pathname !== '/404' ? 200 : 404;
121+
} catch {
122+
return 200;
123+
}
124+
}
125+
126+
function isKnownApplicationRoute(pathname: string): boolean {
127+
return getKnownApplicationRoutePatterns().some(pattern => pattern.test(pathname));
128+
}
129+
130+
function getKnownApplicationRoutePatterns(): RegExp[] {
131+
const routePatterns = [
132+
/^\/$/,
133+
/^\/home\/?$/,
134+
/^\/admin\/?$/,
135+
/^\/auth\/(?:login|signup|reset-password|callback|complete-profile)\/?$/,
136+
/^\/u\/[^/]+\/?$/,
137+
/^\/user\/account\/?$/,
138+
/^\/user\/area\/?$/,
139+
/^\/racks(?:\/browser|\/details\/\d+|\/[^/]+)?\/?$/,
140+
/^\/patches(?:\/browser|\/details\/\d+|\/[^/]+)?\/?$/,
141+
/^\/modules(?:\/browser|\/details\/\d+|\/add)?\/?$/,
142+
/^\/manufacturers(?:\/browser|\/details\/\d+)?\/?$/,
143+
/^\/info\/(?:changelog|insights)\/?$/,
144+
/^\/links\/retired\/?$/,
145+
/^\/404\/?$/,
146+
];
147+
148+
if (environment.features.collectionsEnabled) {
149+
routePatterns.push(
150+
/^\/collections(?:\/browser|\/manage\/[^/]+|\/[^/]+)?\/?$/,
151+
/^\/collection\/[^/]+\/?$/,
152+
);
153+
}
154+
155+
return routePatterns;
156+
}

‎vercel.json‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,13 @@
77
"routes": [
88
{ "src": "/sitemap.xml", "dest": "/api/sitemap" },
99
{ "handle": "filesystem" },
10-
{ "src": "/(.*)", "dest": "/index.html" }
10+
{ "src": "/(.*)", "dest": "/api/ssr" }
1111
],
12+
"functions": {
13+
"api/ssr.mjs": {
14+
"includeFiles": "dist/Patcher/**"
15+
}
16+
},
1217
"headers": [
1318
{
1419
"source": "/:file.js",

0 commit comments

Comments
 (0)