Skip to content

Commit

Permalink
fix(platform-server): surface errors during rendering (#50587)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alan-agius4 authored and alxhub committed Jun 7, 2023
1 parent 7837f71 commit 0875b51
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 42 deletions.
43 changes: 43 additions & 0 deletions 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();
1 change: 1 addition & 0 deletions integration/platform-server/projects/ngmodule/server.ts
Expand Up @@ -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');
Expand Down
Expand Up @@ -26,6 +26,15 @@ const routes: Routes = [
(m) => m.HttpTransferStateOnInitModule
),
},
{
path: 'error',
component: HelloWorldComponent,
resolve: {
'id': () => {
throw new Error('Error in resolver.');
},
},
},
];

@NgModule({
Expand Down
43 changes: 43 additions & 0 deletions 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();
1 change: 1 addition & 0 deletions integration/platform-server/projects/standalone/server.ts
Expand Up @@ -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');
Expand Down

This file was deleted.

Expand Up @@ -25,4 +25,13 @@ export const routes: Routes = [
(c) => c.TransferStateOnInitComponent
),
},
{
path: 'error',
component: HelloWorldComponent,
resolve: {
'id': () => {
throw new Error('Error in resolver.');
},
},
},
];
10 changes: 9 additions & 1 deletion packages/platform-server/src/utils.ts
Expand Up @@ -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<void>((resolve) => {
setTimeout(() => {
platformRef.destroy();
resolve();
}, 0);
});

return output;
}
Expand Down
8 changes: 2 additions & 6 deletions packages/platform-server/test/hydration_spec.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -279,9 +278,6 @@ describe('platform-server integration', () => {
async function hydrate(
html: string, component: Type<unknown>, envProviders?: Provider[],
hydrationFeatures: HydrationFeature<HydrationFeatureKind>[] = []): Promise<ApplicationRef> {
// Destroy existing platform, a new one will be created later by the `bootstrapApplication`.
destroyPlatform();

// Get HTML contents of the `<app>`, 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));
Expand Down Expand Up @@ -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');
Expand Down
17 changes: 10 additions & 7 deletions packages/platform-server/test/integration_spec.ts
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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(
'<html><head><title>RenderHook</title></head><body>' +
'<app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</app></body></html>');
});
const output = await bootstrap;
// title should be added by the render hook.
expect(output).toBe(
'<html><head><title>RenderHook</title></head><body>' +
'<app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</app></body></html>');
});

it('should call multiple render hooks', async () => {
Expand Down

0 comments on commit 0875b51

Please sign in to comment.