From dab36fc60395325fb185433c86b90b289595edd8 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 6 Jun 2023 10:32:39 +0000 Subject: [PATCH] fix(platform-server): surface errors during rendering Prior to this change in some cases errors tht happen during routing were not being surfaced. This is due to the fact that the router has floating promises, and the platform was being destroyed prior to these being settled. --- .../projects/ngmodule/prerender.ts | 43 +++++++++++++++++++ .../projects/ngmodule/server.ts | 1 + .../ngmodule/src/app/app-routing.module.ts | 9 ++++ .../projects/standalone/prerender.ts | 43 +++++++++++++++++++ .../projects/standalone/server.ts | 1 + .../standalone/src/app/app-routing.module.ts | 28 ------------ .../projects/standalone/src/app/app.routes.ts | 9 ++++ packages/platform-server/src/utils.ts | 10 ++++- .../platform-server/test/hydration_spec.ts | 8 +--- .../platform-server/test/integration_spec.ts | 17 +++++--- 10 files changed, 127 insertions(+), 42 deletions(-) create mode 100644 integration/platform-server/projects/ngmodule/prerender.ts create mode 100644 integration/platform-server/projects/standalone/prerender.ts delete mode 100644 integration/platform-server/projects/standalone/src/app/app-routing.module.ts diff --git a/integration/platform-server/projects/ngmodule/prerender.ts b/integration/platform-server/projects/ngmodule/prerender.ts new file mode 100644 index 000000000000..16ac8df13aae --- /dev/null +++ b/integration/platform-server/projects/ngmodule/prerender.ts @@ -0,0 +1,43 @@ +/** + * @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.io/license + */ +/* tslint:disable:no-console */ +import 'zone.js/node'; +import {join} from 'path'; +import {renderModule} from '@angular/platform-server'; +import {readFileSync} from 'fs'; +import {AppServerModule} from './src/main.server'; + +const distFolder = join(process.cwd(), 'dist/ngmodule/browser'); +const indexHtml = readFileSync(join(distFolder, 'index.html'), 'utf-8'); + +async function runTest() { + // Test and validate the errors are printed in the console. + const originalConsoleError = console.error; + const errors: string[] = []; + console.error = (error, data) => errors.push(error.toString() + ' ' + data.toString()); + + try { + await renderModule(AppServerModule, { + document: indexHtml, + url: '/error', + }); + } catch {} + + console.error = originalConsoleError; + + // Test case + if (!errors.some((e) => e.includes('Error in resolver'))) { + errors.forEach(console.error); + console.error( + '\nError: expected rendering errors ("Error in resolver") to be printed in the console.\n' + ); + process.exit(1); + } +} + +runTest(); diff --git a/integration/platform-server/projects/ngmodule/server.ts b/integration/platform-server/projects/ngmodule/server.ts index db0da1840e8f..e908b269c8e7 100644 --- a/integration/platform-server/projects/ngmodule/server.ts +++ b/integration/platform-server/projects/ngmodule/server.ts @@ -13,6 +13,7 @@ import * as express from 'express'; import {AppServerModule} from './src/main.server'; import {join} from 'path'; import {readFileSync} from 'fs'; +import './prerender'; const app = express(); const distFolder = join(process.cwd(), 'dist/ngmodule/browser'); diff --git a/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts b/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts index 15b321b453ca..0ed8c070fea5 100644 --- a/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts +++ b/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts @@ -26,6 +26,15 @@ const routes: Routes = [ (m) => m.HttpTransferStateOnInitModule ), }, + { + path: 'error', + component: HelloWorldComponent, + resolve: { + 'id': () => { + throw new Error('Error in resolver.'); + }, + }, + }, ]; @NgModule({ diff --git a/integration/platform-server/projects/standalone/prerender.ts b/integration/platform-server/projects/standalone/prerender.ts new file mode 100644 index 000000000000..e8197a22a9db --- /dev/null +++ b/integration/platform-server/projects/standalone/prerender.ts @@ -0,0 +1,43 @@ +/** + * @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.io/license + */ +/* tslint:disable:no-console */ +import 'zone.js/node'; +import {renderApplication} from '@angular/platform-server'; +import bootstrap from './src/main.server'; +import {join} from 'path'; +import {readFileSync} from 'fs'; + +const distFolder = join(process.cwd(), 'dist/standalone/browser'); +const indexHtml = readFileSync(join(distFolder, 'index.html'), 'utf-8'); + +async function runTest() { + // Test and validate the errors are printed in the console. + const originalConsoleError = console.error; + const errors: string[] = []; + console.error = (error, data) => errors.push(error.toString() + ' ' + data.toString()); + + try { + await renderApplication(bootstrap, { + document: indexHtml, + url: '/error', + }); + } catch {} + + console.error = originalConsoleError; + + // Test case + if (!errors.some((e) => e.includes('Error in resolver'))) { + errors.forEach(console.error); + console.error( + '\nError: expected rendering errors ("Error in resolver") to be printed in the console.\n' + ); + process.exit(1); + } +} + +runTest(); diff --git a/integration/platform-server/projects/standalone/server.ts b/integration/platform-server/projects/standalone/server.ts index 2437c1655f3d..ac1b37df48fb 100644 --- a/integration/platform-server/projects/standalone/server.ts +++ b/integration/platform-server/projects/standalone/server.ts @@ -13,6 +13,7 @@ import * as express from 'express'; import bootstrap from './src/main.server'; import {join} from 'path'; import {readFileSync} from 'fs'; +import './prerender'; const app = express(); const distFolder = join(process.cwd(), 'dist/standalone/browser'); diff --git a/integration/platform-server/projects/standalone/src/app/app-routing.module.ts b/integration/platform-server/projects/standalone/src/app/app-routing.module.ts deleted file mode 100644 index 701e7bc35a78..000000000000 --- a/integration/platform-server/projects/standalone/src/app/app-routing.module.ts +++ /dev/null @@ -1,28 +0,0 @@ -import {NgModule} from '@angular/core'; -import {RouterModule, Routes} from '@angular/router'; -import {HelloWorldComponent} from './helloworld/hello-world.component'; -import {TransferStateComponent} from './transferstate/transfer-state.component'; - -const routes: Routes = [ - { - path: 'helloworld', - component: HelloWorldComponent, - }, - { - path: 'transferstate', - component: TransferStateComponent, - }, - { - path: 'http-transferstate-lazy', - loadChildren: () => - import('./http-transferstate-lazy/http-transfer-state.module').then( - (m) => m.HttpTransferStateModule - ), - }, -]; - -@NgModule({ - imports: [RouterModule.forRoot(routes)], - exports: [RouterModule], -}) -export class AppRoutingModule {} diff --git a/integration/platform-server/projects/standalone/src/app/app.routes.ts b/integration/platform-server/projects/standalone/src/app/app.routes.ts index 697c7ecb7301..0113034fd44b 100644 --- a/integration/platform-server/projects/standalone/src/app/app.routes.ts +++ b/integration/platform-server/projects/standalone/src/app/app.routes.ts @@ -25,4 +25,13 @@ export const routes: Routes = [ (c) => c.TransferStateOnInitComponent ), }, + { + path: 'error', + component: HelloWorldComponent, + resolve: { + 'id': () => { + throw new Error('Error in resolver.'); + }, + }, + }, ]; diff --git a/packages/platform-server/src/utils.ts b/packages/platform-server/src/utils.ts index 807b983317c7..78ed21c4a3c0 100644 --- a/packages/platform-server/src/utils.ts +++ b/packages/platform-server/src/utils.ts @@ -90,7 +90,15 @@ async function _render(platformRef: PlatformRef, applicationRef: ApplicationRef) appendServerContextInfo(applicationRef); const output = platformState.renderToString(); - platformRef.destroy(); + + // Destroy the application in a macrotask, this allows pending promises to be settled and errors + // to be surfaced to the users. + await new Promise((resolve) => { + setTimeout(() => { + platformRef.destroy(); + resolve(); + }, 0); + }); return output; } diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts index 26a3b8a07031..0b54159d4cd4 100644 --- a/packages/platform-server/test/hydration_spec.ts +++ b/packages/platform-server/test/hydration_spec.ts @@ -12,7 +12,6 @@ import {CommonModule, DOCUMENT, isPlatformServer, NgComponentOutlet, NgFor, NgIf import {MockPlatformLocation} from '@angular/common/testing'; import {ApplicationRef, Component, ComponentRef, createComponent, destroyPlatform, Directive, ElementRef, EnvironmentInjector, ErrorHandler, getPlatform, inject, Injectable, Input, NgZone, PLATFORM_ID, Provider, TemplateRef, Type, ViewChild, ViewContainerRef, ViewEncapsulation, ɵsetDocument} from '@angular/core'; import {Console} from '@angular/core/src/console'; -import {InitialRenderPendingTasks} from '@angular/core/src/initial_render_pending_tasks'; import {getComponentDef} from '@angular/core/src/render3/definition'; import {NoopNgZone} from '@angular/core/src/zone/ng_zone'; import {TestBed} from '@angular/core/testing'; @@ -213,7 +212,7 @@ function withDebugConsole() { return [{provide: Console, useClass: DebugConsole}]; } -describe('platform-server integration', () => { +describe('platform-server hydration integration', () => { beforeEach(() => { if (typeof ngDevMode === 'object') { // Reset all ngDevMode counters. @@ -279,9 +278,6 @@ describe('platform-server integration', () => { async function hydrate( html: string, component: Type, envProviders?: Provider[], hydrationFeatures: HydrationFeature[] = []): Promise { - // Destroy existing platform, a new one will be created later by the `bootstrapApplication`. - destroyPlatform(); - // Get HTML contents of the ``, create a DOM element and append it into the body. const container = convertHtmlToDom(html, doc); Array.from(container.children).forEach(node => doc.body.appendChild(node)); @@ -4777,7 +4773,7 @@ describe('platform-server integration', () => { } } - ssr(SimpleComponent, undefined, withNoopErrorHandler()).catch((err: unknown) => { + await ssr(SimpleComponent, undefined, withNoopErrorHandler()).catch((err: unknown) => { const message = (err as Error).message; expect(message).toContain( 'During serialization, Angular was unable to find an element in the DOM'); diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 8362ea560175..da1cc682cccd 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -532,7 +532,11 @@ if (getDOM().supportsDOMEvents) return; // NODE only describe('platform-server integration', () => { beforeEach(() => { - if (getPlatform()) destroyPlatform(); + destroyPlatform(); + }); + + afterAll(() => { + destroyPlatform(); }); it('should bootstrap', async () => { @@ -994,12 +998,11 @@ describe('platform-server integration', () => { renderApplication( getStandaloneBoostrapFn(MyServerAppStandalone, RenderHookProviders), options) : renderModule(RenderHookModule, options); - bootstrap.then(output => { - // title should be added by the render hook. - expect(output).toBe( - 'RenderHook' + - 'Works!'); - }); + const output = await bootstrap; + // title should be added by the render hook. + expect(output).toBe( + 'RenderHook' + + 'Works!'); }); it('should call multiple render hooks', async () => {