Skip to content

Commit

Permalink
fix: make sure there is a check for targets when timeout=0 (#8765)
Browse files Browse the repository at this point in the history
Previously, if timeout is falsy, the targets would only
be checked if a browser-level event fires which lead to
a race: if the events arrived before waiting for a target,
the promise would never resolve.

Fixes #8763
  • Loading branch information
OrKoN committed Aug 10, 2022
1 parent b5064b7 commit c23cdb7
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/common/Browser.ts
Expand Up @@ -649,10 +649,10 @@ export class Browser extends EventEmitter {
this.on(BrowserEmittedEvents.TargetCreated, check);
this.on(BrowserEmittedEvents.TargetChanged, check);
try {
this.targets().forEach(check);
if (!timeout) {
return await targetPromise;
}
this.targets().forEach(check);
return await waitWithTimeout(targetPromise, 'target', timeout);
} finally {
this.off(BrowserEmittedEvents.TargetCreated, check);
Expand Down
8 changes: 8 additions & 0 deletions test/src/launcher.spec.ts
Expand Up @@ -543,6 +543,14 @@ describe('Launcher specs', function () {
});
expect(error).toBeInstanceOf(puppeteer.errors.TimeoutError);
});
it('should work with timeout = 0', async () => {
const {puppeteer, defaultBrowserOptions} = getTestState();
const options = Object.assign({}, defaultBrowserOptions, {
timeout: 0,
});
const browser = await puppeteer.launch(options);
await browser.close();
});
it('should set the default viewport', async () => {
const {puppeteer, defaultBrowserOptions} = getTestState();
const options = Object.assign({}, defaultBrowserOptions, {
Expand Down

0 comments on commit c23cdb7

Please sign in to comment.