From 0875b519b9dcf15703039b20ef7398b0c964ba0c 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 (#50587) 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. PR Close #50587 --- .../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 0000000000000..16ac8df13aae1 --- /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 db0da1840e8f0..e908b269c8e7b 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 15b321b453caa..0ed8c070fea51 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 0000000000000..e8197a22a9dbf --- /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 2437c1655f3d2..ac1b37df48fb9 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 701e7bc35a78e..0000000000000 --- 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 697c7ecb7301f..0113034fd44b2 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 807b983317c77..78ed21c4a3c02 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 26a3b8a070312..0b54159d4cd47 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 8362ea560175b..da1cc682cccdf 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 () => {