Skip to content

Commit

Permalink
fix(reload): do not throw when reload is racing with navigation (#5113)…
Browse files Browse the repository at this point in the history
… (#5171)

When `page.reload()` is racing against the renderer-initiated
navigation, we might end up with `waitForNavigation()` being rejected
before the reload implementation is able to catch it.

To avoid that, carefully use Promise.all and await `waitForNavigation`
from the get go.

Same happens to `page.goForward()` and `page.goBack()`.
  • Loading branch information
dgozman committed Jan 28, 2021
1 parent 0e216bc commit a1c6ab2
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 13 deletions.
44 changes: 31 additions & 13 deletions src/server/page.ts
Expand Up @@ -293,9 +293,13 @@ export class Page extends EventEmitter {
async reload(controller: ProgressController, options: types.NavigateOptions): Promise<network.Response | null> {
this.mainFrame().setupNavigationProgressController(controller);
const response = await controller.run(async progress => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, options);
await this._delegate.reload();
return waitPromise;
// Note: waitForNavigation may fail before we get response to reload(),
// so we should await it immediately.
const [response] = await Promise.all([
this.mainFrame()._waitForNavigation(progress, options),
this._delegate.reload(),
]);
return response;
}, this._timeoutSettings.navigationTimeout(options));
await this._doSlowMo();
return response;
Expand All @@ -304,13 +308,20 @@ export class Page extends EventEmitter {
async goBack(controller: ProgressController, options: types.NavigateOptions): Promise<network.Response | null> {
this.mainFrame().setupNavigationProgressController(controller);
const response = await controller.run(async progress => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, options);
// Note: waitForNavigation may fail before we get response to goBack,
// so we should catch it immediately.
let error: Error | undefined;
const waitPromise = this.mainFrame()._waitForNavigation(progress, options).catch(e => {
error = e;
return null;
});
const result = await this._delegate.goBack();
if (!result) {
waitPromise.catch(() => {});
if (!result)
return null;
}
return waitPromise;
const response = await waitPromise;
if (error)
throw error;
return response;
}, this._timeoutSettings.navigationTimeout(options));
await this._doSlowMo();
return response;
Expand All @@ -319,13 +330,20 @@ export class Page extends EventEmitter {
async goForward(controller: ProgressController, options: types.NavigateOptions): Promise<network.Response | null> {
this.mainFrame().setupNavigationProgressController(controller);
const response = await controller.run(async progress => {
const waitPromise = this.mainFrame()._waitForNavigation(progress, options);
// Note: waitForNavigation may fail before we get response to goForward,
// so we should catch it immediately.
let error: Error | undefined;
const waitPromise = this.mainFrame()._waitForNavigation(progress, options).catch(e => {
error = e;
return null;
});
const result = await this._delegate.goForward();
if (!result) {
waitPromise.catch(() => {});
if (!result)
return null;
}
return waitPromise;
const response = await waitPromise;
if (error)
throw error;
return response;
}, this._timeoutSettings.navigationTimeout(options));
await this._doSlowMo();
return response;
Expand Down
64 changes: 64 additions & 0 deletions test/page-history.spec.ts
Expand Up @@ -92,3 +92,67 @@ it('page.reload should work with data url', async ({page, server}) => {
expect(await page.reload()).toBe(null);
expect(await page.content()).toContain('hello');
});

it('page.reload during renderer-initiated navigation', async ({page, server}) => {
await page.goto(server.PREFIX + '/one-style.html');
await page.setContent(`<form method='POST' action='/post'>Form is here<input type='submit'></form>`);
server.setRoute('/post', (req, res) => {});

let callback;
const reloadFailedPromise = new Promise(f => callback = f);
page.once('request', async () => {
await page.reload().catch(e => {});
callback();
});
const clickPromise = page.click('input[type=submit]').catch(e => {});
await reloadFailedPromise;
await clickPromise;

// Form submit should be canceled, and reload should eventually arrive
// to the original one-style.html.
await page.waitForSelector('text=hello');
});

it('page.goBack during renderer-initiated navigation', async ({page, server}) => {
await page.goto(server.PREFIX + '/one-style.html');
await page.goto(server.EMPTY_PAGE);
await page.setContent(`<form method='POST' action='/post'>Form is here<input type='submit'></form>`);
server.setRoute('/post', (req, res) => {});

let callback;
const reloadFailedPromise = new Promise(f => callback = f);
page.once('request', async () => {
await page.goBack().catch(e => {});
callback();
});
const clickPromise = page.click('input[type=submit]').catch(e => {});
await reloadFailedPromise;
await clickPromise;

// Form submit should be canceled, and goBack should eventually arrive
// to the original one-style.html.
await page.waitForSelector('text=hello');
});

it('page.goForward during renderer-initiated navigation', async ({page, server}) => {
await page.goto(server.EMPTY_PAGE);
await page.goto(server.PREFIX + '/one-style.html');
await page.goBack();

await page.setContent(`<form method='POST' action='/post'>Form is here<input type='submit'></form>`);
server.setRoute('/post', (req, res) => {});

let callback;
const reloadFailedPromise = new Promise(f => callback = f);
page.once('request', async () => {
await page.goForward().catch(e => {});
callback();
});
const clickPromise = page.click('input[type=submit]').catch(e => {});
await reloadFailedPromise;
await clickPromise;

// Form submit should be canceled, and goForward should eventually arrive
// to the original one-style.html.
await page.waitForSelector('text=hello');
});

0 comments on commit a1c6ab2

Please sign in to comment.