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

fix: suppress init errors if the target is closed #8947

Merged
merged 1 commit into from Sep 14, 2022
Merged

fix: suppress init errors if the target is closed #8947

merged 1 commit into from Sep 14, 2022

Conversation

OrKoN
Copy link
Collaborator

@OrKoN OrKoN commented Sep 13, 2022

With #8520 Puppeteer is now aware of all targets it connects to. In order to have a not flaky init, Puppeteer waits for all existing targets to be configured during the connection process. This does not work well in case of concurrent connections because while one connection might initialising a target the other one might be closed it. In general, that is expected because we can only be eventually consistent about the target state but we also should not crash the init if some targets have been closed. This PR implements checks to see if the errors are caused by the target or session closures and suppresses them if it's the case.

Also we don't need to call browser.pages() in BrowserConnector as it appears to be redundant for the current TargetManager implementation and forces connection to all existing pages.

Closes #8944

@OrKoN OrKoN force-pushed the fixed-8944 branch 2 times, most recently from 41474a1 to d15e8c3 Compare September 13, 2022 15:29
@OrKoN OrKoN requested a review from jrandolf September 13, 2022 15:31
@OrKoN OrKoN marked this pull request as ready for review September 13, 2022 15:31
src/common/Page.ts Outdated Show resolved Hide resolved
@jrandolf jrandolf changed the title fix: supress init errors if the target is closed fix: suppress init errors if the target is closed Sep 14, 2022
With #8520 Puppeteer is now aware of all targets it connects
to. In order to have a not flaky init, Puppeteer waits for
all existing targets to be configured during the connection process.
This does not work well in case of concurrent connections because
while one connection might initializing a target the other one
might be closed it. In general, that is expected because we
can only be eventually consistent about the target state but we
also should not crash the init if some targets have been closed.
This PR implements checks to see if the errors are caused by the
target or session closures and supresses them if it's the case.
@OrKoN OrKoN enabled auto-merge (squash) September 14, 2022 11:23
@OrKoN OrKoN merged commit a4ec7de into main Sep 14, 2022
@OrKoN OrKoN deleted the fixed-8944 branch September 14, 2022 12:18
jrandolf pushed a commit that referenced this pull request Sep 15, 2022
With #8520 Puppeteer is now aware of all targets it connects
to. In order to have a not flaky init, Puppeteer waits for
all existing targets to be configured during the connection process.
This does not work well in case of concurrent connections because
while one connection might initializing a target the other one
might be closed it. In general, that is expected because we
can only be eventually consistent about the target state but we
also should not crash the init if some targets have been closed.
This PR implements checks to see if the errors are caused by the
target or session closures and suppresses them if it's the case.
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.

[Bug]: Protocol error when calling puppeteer.connect()
2 participants