Skip to content

Commit

Permalink
fix(platform/messaging): stop platform in beforeunload to avoid pos…
Browse files Browse the repository at this point in the history
…ting messages to disposed window

When posting a message to a disposed window, the browser logs "Failed to execute 'postMessage' on 'DOMWindow'" error.
To avoid posting messages to disposed windows, we now disconnect clients already in `beforeunload` instead of `unload`.
However, if `beforeunload` is not triggered, e.g., when an iframe is removed, we fall back to `unload`.

closes #168
  • Loading branch information
danielwiehl authored and mofogasy committed Nov 11, 2022
1 parent 09b682b commit 3969c17
Show file tree
Hide file tree
Showing 12 changed files with 217 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const routes: Routes = [
{path: 'scrollable-microfrontend', component: ScrollableMicrofrontendComponent, data: {pageTitle: 'Displays a microfrontend with some tall content displayed in a viewport'}},
{path: 'preferred-size', component: PreferredSizeComponent, data: {pageTitle: 'Allows playing around with the microfrontend\'s preferred size'}},
{path: 'platform-properties', component: PlatformPropertiesComponent, data: {pageTitle: 'Shows properties that are registered in the platform'}},
{path: 'test-pages', loadChildren: (): any => import('./test-pages/test-pages-routing.module').then(m => m.TestPagesRoutingModule)},
],
},
];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<form autocomplete="off" [formGroup]="form">
<sci-form-field label="Router Outlet">
<input [formControlName]="OUTLET" class="e2e-outlet" title="Specifies the name of the outlet which to clear.">
</sci-form-field>
<sci-form-field label="Message Topic">
<input [formControlName]="TOPIC" class="e2e-topic" title="Specifies the topic where to send a message after cleared the outlet.">
</sci-form-field>

<button type="submit" class="e2e-run-test" [disabled]="form.invalid" (click)="onRunTestClick()">
Clear outlet, then send message to topic
</button>
</form>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
:host {
display: grid;

> form {
display: grid;
grid-template-columns: 1fr;
grid-auto-rows: max-content;
row-gap: .25em;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright (c) 2018-2022 Swiss Federal Railways
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/
import {Component} from '@angular/core';
import {FormControl, FormGroup, ReactiveFormsModule, UntypedFormBuilder, Validators} from '@angular/forms';
import {MessageClient, OutletRouter} from '@scion/microfrontend-platform';
import {Beans} from '@scion/toolkit/bean-manager';
import {SciFormFieldModule} from '@scion/components.internal/form-field';

export const OUTLET = 'outlet';
export const TOPIC = 'topic';

@Component({
selector: 'app-clear-outlet-then-send-message-test-page',
templateUrl: './clear-outlet-then-send-message-test-page.component.html',
styleUrls: ['./clear-outlet-then-send-message-test-page.component.scss'],
standalone: true,
imports: [ReactiveFormsModule, SciFormFieldModule],
})
export class ClearOutletThenSendMessageTestPageComponent {

public OUTLET = OUTLET;
public TOPIC = TOPIC;

public form: FormGroup;

constructor(formBuilder: UntypedFormBuilder) {
this.form = formBuilder.group({
[OUTLET]: new FormControl<string>('', Validators.required),
[TOPIC]: new FormControl<string>('', Validators.required),
});
}

public async onRunTestClick(): Promise<void> {
// Clear the router outlet.
await Beans.get(OutletRouter).navigate(null, {outlet: this.form.get(OUTLET).value});
// Send message to the topic.
await Beans.get(MessageClient).publish(this.form.get(TOPIC).value);
// Reset the form.
this.form.reset();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2018-2022 Swiss Federal Railways
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/

import {NgModule} from '@angular/core';
import {RouterModule, Routes} from '@angular/router';

const routes: Routes = [
{
path: 'clear-outlet-then-send-message-test-page',
data: {pageTitle: 'Test page that clears an outlet and sends a message'},
loadComponent: (): any => import('./clear-outlet-then-send-message-test-page/clear-outlet-then-send-message-test-page.component').then(m => m.ClearOutletThenSendMessageTestPageComponent),
},
];

@NgModule({
imports: [RouterModule.forChild(routes)],
exports: [RouterModule],
})
export class TestPagesRoutingModule {
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ When the platform is stopped, it first enters the `Stopping` platform state. In

The platform allows the application to send messages while the platform is stopping. After all beans are destroyed, the client is disconnected from the host and the platform enters the `Stopped` state.

By default, the platform initiates platform shutdown when the browser unloads the document. For this purpose, the platform binds to the browser's `unload` event. It does not bind to the `beforeunload` event since the browser fires that event only when navigating to another page, but not when removing the iframe.
By default, the platform initiates shutdown when the browser unloads the document, i.e., when `beforeunload` is triggered. The main reason for `beforeunload` instead of `unload` is to avoid posting messages to disposed windows. However, if `beforeunload` is not triggered, e.g., when an iframe is removed, we fall back to `unload`.

You can change this behavior by registering a custom `MicrofrontendPlatformStopper` bean, as follows:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ import {MicrofrontendPlatform, MicrofrontendPlatformStopper, PlatformState} from

{
// tag::platform-lifecycle:microfrontend-platform-stopper[]
class BeforeUnloadMicrofrontendPlatformStopper implements MicrofrontendPlatformStopper {
class OnUnloadMicrofrontendPlatformStopper implements MicrofrontendPlatformStopper {

constructor() {
// Destroys the platform when the document is about to be unloaded.
window.addEventListener('beforeunload', () => MicrofrontendPlatform.destroy(), {once: true});
window.addEventListener('unload', () => MicrofrontendPlatform.destroy(), {once: true});
}
}

// Registers custom platform stopper.
Beans.register(MicrofrontendPlatformStopper, {useClass: BeforeUnloadMicrofrontendPlatformStopper});
Beans.register(MicrofrontendPlatformStopper, {useClass: OnUnloadMicrofrontendPlatformStopper});
// end::platform-lifecycle:microfrontend-platform-stopper[]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ test.describe('Messaging', () => {
await TopicBasedMessagingSpecs.passHeadersSpec(testingAppPO);
});

test('should stop platform in `beforeunload` to avoid posting messages to disposed windows', async ({testingAppPO, consoleLogs}) => {
await TopicBasedMessagingSpecs.doNotPostMessageToDisposedWindow(testingAppPO, consoleLogs);
});

test.describe('message-interception', () => {

test('allows intercepting messages', async ({testingAppPO}) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {OutletPageObject} from '../browser-outlet/browser-outlet.po';

export class ReceiveMessagePagePO implements OutletPageObject {

public readonly path = 'receive-message';
public static readonly PATH = 'receive-message';
public readonly path = ReceiveMessagePagePO.PATH;

private readonly _locator: Locator;
private readonly _messageListPO: SciListPO;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import {MessagingFlavor, PublishMessagePagePO} from './publish-message-page.po';
import {ReceiveMessagePagePO} from './receive-message-page.po';
import {BrowserOutletPO} from '../browser-outlet/browser-outlet.po';
import {expect} from '@playwright/test';
import {OutletRouterPagePO} from '../router-outlet/outlet-router-page.po';
import {RouterOutletPagePO} from '../router-outlet/router-outlet-page.po';
import {ClearOutletThenSendMessageTestPagePO} from '../test-pages/clear-outlet-then-send-message-test-page.po';
import {ConsoleLogs} from '../console-logs';

/**
* Contains Specs for topic-based messaging.
Expand Down Expand Up @@ -687,6 +691,59 @@ export namespace TopicBasedMessagingSpecs {
});
}

/**
* Tests to not post messages to disposed windows, causing "Failed to execute 'postMessage' on 'DOMWindow'" error.
*
* The error occurred when posting a message to a microfrontend that was unloaded, but before the disconnect event arrived in the broker.
* To reproduce the bug, we subscribe to messages in a router outlet, unload it, and send a message right after.
*/
export async function doNotPostMessageToDisposedWindow(testingAppPO: TestingAppPO, consoleLogs: ConsoleLogs): Promise<void> {
const pagePOs = await testingAppPO.navigateTo({
testpage: ClearOutletThenSendMessageTestPagePO,
router: OutletRouterPagePO,
receiver1: {useClass: RouterOutletPagePO, origin: TestingAppOrigins.APP_2},
receiver2: {useClass: RouterOutletPagePO, origin: TestingAppOrigins.APP_3},
});

// Mount router outlet.
const routerOutlet1PO = pagePOs.get<RouterOutletPagePO>('receiver1');
await routerOutlet1PO.enterOutletName('test-outlet');
await routerOutlet1PO.clickApply();

// Mount router outlet.
const routerOutlet2PO = pagePOs.get<RouterOutletPagePO>('receiver2');
await routerOutlet2PO.enterOutletName('test-outlet');
await routerOutlet2PO.clickApply();

// Load message receiver into the outlets.
const routerPO = pagePOs.get<OutletRouterPagePO>('router');
await routerPO.enterOutletName('test-outlet');
await routerPO.enterUrl(`../${ReceiveMessagePagePO.PATH}`);
await routerPO.clickNavigate();

// Subscribe to messages in the outlet.
const receiver1PO = new ReceiveMessagePagePO(routerOutlet1PO.routerOutletFrameLocator);
await receiver1PO.enterTopic('test-topic');
await receiver1PO.clickSubscribe();

// Subscribe to messages in the outlet.
const receiver2PO = new ReceiveMessagePagePO(routerOutlet2PO.routerOutletFrameLocator);
await receiver2PO.enterTopic('test-topic');
await receiver2PO.clickSubscribe();

// Run the test to provoke the error.
// 1. Unload the outlet that has loaded a microfrontend subscribed to messages.
// 2. Send a message to the subscribed topic.
const testepagePO = pagePOs.get<ClearOutletThenSendMessageTestPagePO>('testpage');
await testepagePO.enterOutletName('test-outlet');
await testepagePO.enterTopic('test-topic');
await testepagePO.clickRunTest();

// Expect no error to be thrown
const errors = await consoleLogs.get({filter: /Failed to execute 'postMessage' on 'DOMWindow'/, severity: 'error'});
await expect(errors).toEqual([]);
}

/**
* Tests message interception by changing the message body to upper case characters.
* The testing app is configured to uppercase messages sent to the topic 'uppercase'.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2018-2022 Swiss Federal Railways
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/

import {FrameLocator, Locator} from '@playwright/test';
import {OutletPageObject} from '../browser-outlet/browser-outlet.po';

export class ClearOutletThenSendMessageTestPagePO implements OutletPageObject {

public readonly path = 'test-pages/clear-outlet-then-send-message-test-page';

private readonly _locator: Locator;

constructor(frameLocator: FrameLocator) {
this._locator = frameLocator.locator('app-clear-outlet-then-send-message-test-page');
}

public async enterOutletName(outlet: string): Promise<void> {
await this._locator.locator('input.e2e-outlet').fill(outlet);
}

public async enterTopic(topic: string): Promise<void> {
await this._locator.locator('input.e2e-topic').fill(topic);
}

public async clickRunTest(): Promise<void> {
await this._locator.locator('button.e2e-run-test').click();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@
*/
import {Beans, PreDestroy} from '@scion/toolkit/bean-manager';
import {MicrofrontendPlatformRef} from './microfrontend-platform-ref';
import {fromEvent, race, Subject} from 'rxjs';
import {take, takeUntil} from 'rxjs/operators';

/**
* Stops the platform and disconnect this client from the host when the document is being unloaded.
* Stops the platform and disconnects this client from the host when the browser unloads the document.
*
* For this purpose, this class binds to the browser's `unload` event. It does not bind to the `beforeunload`
* event since the browser fires that event only when navigating to another page, but not when removing the iframe.
* By default, the platform initiates shutdown when the browser unloads the document, i.e., when `beforeunload` is triggered.
* The main reason for `beforeunload` instead of `unload` is to avoid posting messages to disposed windows.
* However, if `beforeunload` is not triggered, e.g., when an iframe is removed, we fall back to `unload`.
*/
export abstract class MicrofrontendPlatformStopper {
}
Expand All @@ -24,13 +27,20 @@ export abstract class MicrofrontendPlatformStopper {
*/
export class ɵMicrofrontendPlatformStopper implements MicrofrontendPlatformStopper, PreDestroy {

private onUnload = (): void => Beans.get(MicrofrontendPlatformRef).destroy();
private _destroy$ = new Subject<void>();

constructor() { // eslint-disable-line @typescript-eslint/member-ordering
window.addEventListener('unload', this.onUnload, {once: true});
constructor() {
race(fromEvent(window, 'beforeunload'), fromEvent(window, 'unload'))
.pipe(
take(1),
takeUntil(this._destroy$),
)
.subscribe(() => {
Beans.get(MicrofrontendPlatformRef).destroy();
});
}

public preDestroy(): void {
window.removeEventListener('unload', this.onUnload);
this._destroy$.next();
}
}

0 comments on commit 3969c17

Please sign in to comment.