Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix(remote): make sure api calls reject before browser is closed (#31858)

Browse source 
Upon calling `browser.close()` or dropping remote connection, make sure
to reject api calls before resolving `browser.close()` and firing a
`disconnected` event.

This change aligns the order guarantee with non-remote case.
This commit is contained in:
Dmitry Gozman 2024-07-25 09:57:34 -07:00 committed by GitHub
parent 0c6ecf8df4
commit d8d5289e86
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 30 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
packages/playwright-core/src/client/browserType.ts
View file

@ -151,8 +151,14 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
page._onClose();
context._onClose();
}
browser?._didClose();
connection.close(reason || closeError);
// Give a chance to any API call promises to reject upon page/context closure.
// This happens naturally when we receive page.onClose and browser.onClose from the server
// in separate tasks. However, upon pipe closure we used to dispatch them all synchronously
// here and promises did not have a chance to reject.
// The order of rejects vs closure is a part of the API contract and our test runner
// relies on it to attribute rejections to the right test.
setTimeout(() => browser?._didClose(), 0);
};
pipe.on('closed', params => onPipeClosed(params.reason));
connection.onmessage = message => this._wrapApiCall(() => pipe.send({ message }).catch(() => onPipeClosed()), /* isInternal */ true);

23
tests/library/browsertype-connect.spec.ts
View file

@ -408,6 +408,29 @@ for (const kind of ['launchServer', 'run-server'] as const) {
}
});
test('should reject waitForEvent before browser.close finishes', async ({ connect, startRemoteServer, server }) => {
const remoteServer = await startRemoteServer(kind);
const browser = await connect(remoteServer.wsEndpoint());
const newPage = await browser.newPage();
let rejected = false;
const promise = newPage.waitForEvent('download').catch(() => rejected = true);
await browser.close();
expect(rejected).toBe(true);
await promise;
});
test('should reject waitForEvent before browser.onDisconnect fires', async ({ connect, startRemoteServer, server }) => {
const remoteServer = await startRemoteServer(kind);
const browser = await connect(remoteServer.wsEndpoint());
const newPage = await browser.newPage();
const log: string[] = [];
const promise = newPage.waitForEvent('download').catch(() => log.push('rejected'));
browser.on('disconnected', () => log.push('disconnected'));
await remoteServer.close();
await promise;
await expect.poll(() => log).toEqual(['rejected', 'disconnected']);
});
test('should respect selectors', async ({ playwright, connect, startRemoteServer }) => {
const remoteServer = await startRemoteServer(kind);