diff --git a/packages/angular/ssr/node/src/app-engine.ts b/packages/angular/ssr/node/src/app-engine.ts index 89e6a942856b..4c1baa390a11 100644 --- a/packages/angular/ssr/node/src/app-engine.ts +++ b/packages/angular/ssr/node/src/app-engine.ts @@ -59,6 +59,9 @@ export class AngularNodeAppEngine { * @param requestContext - Optional context for rendering, such as metadata associated with the request. * @returns A promise that resolves to the resulting HTTP response object, or `null` if no matching Angular route is found. * + * @throws {DOMException} With name `'AbortError'` if the request is aborted before rendering completes. + * When aborted, the underlying Angular platform is also destroyed to stop any in-progress rendering. + * * @remarks A request to `https://www.example.com/page/index.html` will serve or render the Angular route * corresponding to `https://www.example.com/page`. * diff --git a/packages/angular/ssr/src/app-engine.ts b/packages/angular/ssr/src/app-engine.ts index 1fa69c1f2629..8afa2bbad822 100644 --- a/packages/angular/ssr/src/app-engine.ts +++ b/packages/angular/ssr/src/app-engine.ts @@ -144,6 +144,9 @@ export class AngularAppEngine { * @param requestContext - Optional context for rendering, such as metadata associated with the request. * @returns A promise that resolves to the resulting HTTP response object, or `null` if no matching Angular route is found. * + * @throws {DOMException} With name `'AbortError'` if `request.signal` is aborted before rendering completes. + * When aborted, the underlying Angular platform is also destroyed to stop any in-progress rendering. + * * @remarks A request to `https://www.example.com/page/index.html` will serve or render the Angular route * corresponding to `https://www.example.com/page`. * diff --git a/packages/angular/ssr/src/app.ts b/packages/angular/ssr/src/app.ts index 57370136f0af..b2dd91af1bba 100644 --- a/packages/angular/ssr/src/app.ts +++ b/packages/angular/ssr/src/app.ts @@ -147,6 +147,9 @@ export class AngularServerApp { * @param requestContext - Optional context for rendering, such as metadata associated with the request. * @returns A promise that resolves to the resulting HTTP response object, or `null` if no matching Angular route is found. * + * @throws {DOMException} With name `'AbortError'` if `request.signal` is aborted before rendering completes. + * When aborted, the underlying Angular platform is also destroyed to stop any in-progress rendering. + * * @remarks A request to `https://www.example.com/page/index.html` will serve or render the Angular route * corresponding to `https://www.example.com/page`. */ @@ -323,6 +326,7 @@ export class AngularServerApp { url, platformProviders, SERVER_CONTEXT_VALUE[renderMode], + request.signal, ); if (result.hasNavigationError) { @@ -349,13 +353,11 @@ export class AngularServerApp { controller.enqueue(this.textEncoder.encode(renderedHtml)); controller.close(); } catch (error) { - result.destroy(); + await result.destroy(); controller.error(error); } }, - cancel: () => { - result.destroy(); - }, + cancel: () => result.destroy(), }); return new Response(stream, responseInit); diff --git a/packages/angular/ssr/src/utils/ng.ts b/packages/angular/ssr/src/utils/ng.ts index 028136749b17..fe1879a17930 100644 --- a/packages/angular/ssr/src/utils/ng.ts +++ b/packages/angular/ssr/src/utils/ng.ts @@ -47,10 +47,12 @@ export type AngularBootstrap = * @param url - The application URL, used for route-based rendering in SSR. * @param platformProviders - An array of platform providers for the rendering process. * @param serverContext - A string representing the server context, providing additional metadata for SSR. + * @param signal - An optional `AbortSignal` to abort the rendering process and destroy the platform. * @returns A promise resolving to an object containing: - * - `hasNavigationError`: Indicates if a navigation error occurred. + * - `hasNavigationError`: Indicates if a navigation error occurred or if rendering was aborted/destroyed before stability. * - `redirectTo`: (Optional) The redirect URL if a navigation redirect occurred. - * - `content`: A function returning a promise that resolves to the rendered HTML string. + * - `content`: (Optional) A function returning a promise that resolves to the rendered HTML string. + * - `destroy`: (Optional) A function to destroy the underlying Angular platform. */ export async function renderAngular( html: string, @@ -58,15 +60,20 @@ export async function renderAngular( url: URL, platformProviders: StaticProvider[], serverContext: string, + signal?: AbortSignal, ): Promise< | { hasNavigationError: true } | { hasNavigationError: boolean; redirectTo?: string; content: () => Promise; - destroy: () => void; + destroy: () => Promise; } > { + if (signal?.aborted) { + return { hasNavigationError: true }; + } + // A request to `http://www.example.com/page/index.html` will render the Angular route corresponding to `http://www.example.com/page`. const urlToRender = stripIndexHtmlFromURL(url); const platformRef = platformServer([ @@ -94,14 +101,46 @@ export async function renderAngular( let redirectTo: string | undefined; let hasNavigationError = true; + let renderTimeoutId: ReturnType | undefined; + let destroyPromise: Promise | undefined; + let rejectContent: ((reason?: unknown) => void) | undefined; + + const destroy = (): Promise => { + rejectContent?.( + new DOMException( + `Request for: ${url} was aborted.${signal?.reason ? `\n${signal.reason}` : ''}`, + 'AbortError', + ), + ); + rejectContent = undefined; + + if (!destroyPromise) { + clearTimeout(renderTimeoutId); + signal?.removeEventListener('abort', abortHandler); + destroyPromise = asyncDestroyPlatform(platformRef); + } + + return destroyPromise; + }; + + const abortHandler = () => void destroy(); try { + signal?.addEventListener('abort', abortHandler, { once: true }); + let applicationRef: ApplicationRef; if (isNgModule(bootstrap)) { const moduleRef = await platformRef.bootstrapModule(bootstrap); + if (platformRef.destroyed || signal?.aborted) { + return { hasNavigationError: true }; + } + applicationRef = moduleRef.injector.get(ApplicationRef); } else { applicationRef = await bootstrap({ platformRef }); + if (platformRef.destroyed || signal?.aborted) { + return { hasNavigationError: true }; + } } // Block until application is stable. @@ -110,7 +149,7 @@ export async function renderAngular( // This code protect against app destruction during bootstrapping which is a // valid case. We should not assume the `applicationRef` is not in destroyed state. // Calling `envInjector.get` would throw `NG0205: Injector has already been destroyed`. - if (applicationRef.destroyed) { + if (applicationRef.destroyed || signal?.aborted) { return { hasNavigationError: true }; } @@ -123,8 +162,6 @@ export async function renderAngular( if (!routerIsProvided) { hasNavigationError = false; } else if (lastSuccessfulNavigation?.finalUrl) { - hasNavigationError = false; - const requestPrefix = envInjector.get(APP_BASE_HREF, null, { optional: true }) ?? getFirstHeaderValue( @@ -138,30 +175,39 @@ export async function renderAngular( if (urlToRenderString !== finalUrl) { redirectTo = [pathname, search, hash].join(''); } + + hasNavigationError = false; } return { - destroy: () => void asyncDestroyPlatform(platformRef), + destroy, hasNavigationError, redirectTo, content: () => new Promise((resolve, reject) => { + rejectContent = reject; + // Defer rendering to the next event loop iteration to avoid blocking, as most operations in `renderInternal` are synchronous. - setTimeout(() => { + renderTimeoutId = setTimeout(() => { + if (platformRef.destroyed || applicationRef.destroyed || signal?.aborted) { + void destroy(); + + return; + } + renderInternal(platformRef, applicationRef) .then(resolve) .catch(reject) - .finally(() => void asyncDestroyPlatform(platformRef)); + .finally(() => { + rejectContent = undefined; + void destroy(); + }); }, 0); }), }; - } catch (error) { - await asyncDestroyPlatform(platformRef); - - throw error; } finally { if (hasNavigationError || redirectTo) { - void asyncDestroyPlatform(platformRef); + void destroy(); } } } diff --git a/packages/angular/ssr/src/utils/promise.ts b/packages/angular/ssr/src/utils/promise.ts index 8129ebdb2df3..ea7fc52da92e 100644 --- a/packages/angular/ssr/src/utils/promise.ts +++ b/packages/angular/ssr/src/utils/promise.ts @@ -17,7 +17,7 @@ * @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. + * @throws {DOMException} With name `'AbortError'` if the `AbortSignal` is triggered before the `promise` resolves. */ export function promiseWithAbort( promise: Promise, @@ -34,12 +34,10 @@ export function promiseWithAbort( // Check for abort signal if (signal.aborted) { abortHandler(); - - return; + } else { + signal.addEventListener('abort', abortHandler, { once: true }); } - signal.addEventListener('abort', abortHandler, { once: true }); - promise .then(resolve) .catch(reject) diff --git a/packages/angular/ssr/test/app_spec.ts b/packages/angular/ssr/test/app_spec.ts index ee9fb9dcc1a5..b50557f1b031 100644 --- a/packages/angular/ssr/test/app_spec.ts +++ b/packages/angular/ssr/test/app_spec.ts @@ -12,7 +12,15 @@ import '@angular/compiler'; /* eslint-enable import/no-unassigned-import */ import { APP_BASE_HREF } from '@angular/common'; -import { Component, PlatformRef, REQUEST, RESPONSE_INIT, inject } from '@angular/core'; +import { + Component, + DestroyRef, + PendingTasks, + PlatformRef, + REQUEST, + RESPONSE_INIT, + inject, +} from '@angular/core'; import { ActivatedRoute, CanActivateFn, Router } from '@angular/router'; import { setTimeout } from 'node:timers/promises'; import { AngularServerApp } from '../src/app'; @@ -21,6 +29,7 @@ import { setAngularAppTestingManifest } from './testing-utils'; describe('AngularServerApp', () => { let app: AngularServerApp; + let slowRouteHook: (() => void) | undefined; @Component({ selector: 'app-home', @@ -28,9 +37,13 @@ describe('AngularServerApp', () => { }) class HomeComponent { constructor() { - if (inject(ActivatedRoute).snapshot.data['destroyApp']) { + const data = inject(ActivatedRoute).snapshot.data; + if (data['destroyApp']) { inject(PlatformRef).destroy(); } + if (data['slow']) { + slowRouteHook?.(); + } } } @@ -78,6 +91,7 @@ describe('AngularServerApp', () => { { path: 'page-with-headers', component: HomeComponent }, { path: 'page-with-status', component: HomeComponent }, { path: 'page-destroy-app', component: HomeComponent, data: { destroyApp: true } }, + { path: 'page-slow', component: HomeComponent, data: { slow: true } }, { path: 'redirect', redirectTo: 'home' }, { path: 'redirect-via-navigate', component: RedirectComponent }, { @@ -313,14 +327,106 @@ describe('AngularServerApp', () => { controller.abort(); }); + await expectAsync(app.handle(request)).toBeRejectedWithError( + DOMException, + /Request for: http:\/\/localhost\/home was aborted/, + ); + }); + + it('should destroy the platform and stop rendering when an in-flight request is aborted', async () => { + let resolveTaskReady!: () => void; + const taskReady = new Promise((resolve) => { + resolveTaskReady = resolve; + }); + let appDestroyed = false; + let destroySpy: jasmine.Spy | undefined; + + slowRouteHook = () => { + destroySpy = spyOn(inject(PlatformRef), 'destroy').and.callThrough(); + const pendingTasks = inject(PendingTasks); + const removeTask = pendingTasks.add(); + inject(DestroyRef).onDestroy(() => { + appDestroyed = true; + removeTask(); + }); + resolveTaskReady(); + }; + try { - await app.handle(request); - throw new Error('Should not be called.'); - } catch (e) { - expect(e).toBeInstanceOf(DOMException); + const controller = new AbortController(); + const request = new Request('http://localhost/page-slow', { signal: controller.signal }); + + const handlePromise = app.handle(request); + + // Wait until the component is instantiated and holding a pending task + await taskReady; + expect(appDestroyed).toBeFalse(); + + // Abort the request while `applicationRef.whenStable()` is still pending + controller.abort(); + + await expectAsync(handlePromise).toBeRejectedWithError( + DOMException, + /Request for: http:\/\/localhost\/page-slow was aborted/, + ); + + // Wait for the macrotask queue to clear since platform destruction is scheduled via setTimeout(0) + await setTimeout(0); + expect(destroySpy).toHaveBeenCalledTimes(1); + expect(appDestroyed).toBeTrue(); + } finally { + slowRouteHook = undefined; } }); + it('should not bootstrap the platform if the request signal is already aborted', async () => { + // Warm up the router so route extraction does not create a PlatformRef during this test + const warmupResponse = await app.handle(new Request('http://localhost/home')); + await warmupResponse?.text(); + await setTimeout(0); + + const destroySpy = spyOn(PlatformRef.prototype, 'destroy').and.callThrough(); + const controller = new AbortController(); + controller.abort(); + + const request = new Request('http://localhost/home', { signal: controller.signal }); + await expectAsync(app.handle(request)).toBeRejectedWithError(DOMException); + + await setTimeout(0); + expect(destroySpy).not.toHaveBeenCalled(); + }); + + it('should clean up the abort event listener on request.signal after rendering completes', async () => { + const controller = new AbortController(); + const request = new Request('http://localhost/home', { signal: controller.signal }); + const removeEventListenerSpy = spyOn( + request.signal, + 'removeEventListener', + ).and.callThrough(); + + const response = await app.handle(request); + expect(await response?.text()).toContain('Home works'); + await setTimeout(0); + + expect(removeEventListenerSpy).toHaveBeenCalledWith('abort', jasmine.any(Function)); + }); + + it('should reject reading response body if request is aborted after handle() resolves but before rendering completes', async () => { + const controller = new AbortController(); + const response = await app.handle( + new Request('http://localhost/home', { signal: controller.signal }), + ); + expect(response).not.toBeNull(); + + controller.abort(); + + await expectAsync(response?.text()).toBeRejectedWithError( + DOMException, + /Request for: http:\/\/localhost\/home was aborted/, + ); + await setTimeout(0); + }); + it('should return configured headers for pages with specific header settings', async () => { const response = await app.handle(new Request('http://localhost/page-with-headers')); const headers = response?.headers.entries() ?? []; diff --git a/packages/angular/ssr/test/utils/promise_spec.ts b/packages/angular/ssr/test/utils/promise_spec.ts index 5f33df85e71f..21172441d314 100644 --- a/packages/angular/ssr/test/utils/promise_spec.ts +++ b/packages/angular/ssr/test/utils/promise_spec.ts @@ -33,4 +33,27 @@ describe('promiseWithAbort', () => { await expectAsync(promise).toBeResolved(); }); + + it('should reject with an AbortError and handle subsequent promise rejection when signal is already aborted', async () => { + const onUnhandledRejection = jasmine.createSpy('unhandledRejection'); + process.on('unhandledRejection', onUnhandledRejection); + + try { + const abortController = new AbortController(); + abortController.abort('Already aborted'); + + const { promise: underlyingPromise, reject } = Promise.withResolvers(); + const promise = promiseWithAbort(underlyingPromise, abortController.signal, 'Test operation'); + + await expectAsync(promise).toBeRejectedWithError(DOMException, /Test operation was aborted/); + + // Rejecting the underlying promise after abort should not cause an unhandled rejection + reject(new Error('Late failure')); + await setTimeout(0); + + expect(onUnhandledRejection).not.toHaveBeenCalled(); + } finally { + process.off('unhandledRejection', onUnhandledRejection); + } + }); }); diff --git a/tsconfig-build-esm.json b/tsconfig-build-esm.json index d70bbf8b2568..5d44015a9631 100644 --- a/tsconfig-build-esm.json +++ b/tsconfig-build-esm.json @@ -8,7 +8,6 @@ "compilerOptions": { "module": "esnext", "target": "es2022", - "lib": ["es2022"], "moduleResolution": "bundler", // don't auto-discover @types/node, it results in a ///