Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't wait for Page.navigate's resolution #3413

Merged
merged 2 commits into from
Sep 26, 2017
Merged

Don't wait for Page.navigate's resolution #3413

merged 2 commits into from
Sep 26, 2017

Conversation

paulirish
Copy link
Member

to repro:

yarn start -- --disable-device-emulation --disable-cpu-throttling --disable-network-throttling "http://example.com" --verbose

this will always timeout at 30 waiting for example.com

that's because it fires loadEventFired before page.navigate ever resolves:

image

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM nice detective work!

.then(_ => this.sendCommand('Page.navigate', {url}))
.then(_ => {
// These can 'race' and that's OK.
// We don't want to wait for Page.navigate's resolution, as it can happen _after_ onload.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a bug reference

@paulirish
Copy link
Member Author

nice detective work!

i noticed example.com was timing out. patrick picked up on the race issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants