Skip to content

chore: fix PlaywrightClient disconnection logic#9149

Merged
mxschmitt merged 2 commits into
microsoft:masterfrom
mxschmitt:test/unflake-selectors-register
Sep 28, 2021
Merged

chore: fix PlaywrightClient disconnection logic#9149
mxschmitt merged 2 commits into
microsoft:masterfrom
mxschmitt:test/unflake-selectors-register

Conversation

@mxschmitt
Copy link
Copy Markdown
Contributor

@mxschmitt mxschmitt commented Sep 25, 2021

The reason was that the selector channels were not unregistered like for BrowserType.connect:

browser.on(Events.Browser.Disconnected, () => {
playwright._cleanup();
closePipe();
});

This resulted that a method on a channel was called which was already closed (ws connection) (then the response was never received and it stalled):

for (const channel of this._channels)
await channel._channel.register(params);

To prevent this in the future, I've added a check to verify that the WS connection (PlaywrightClient) is not CLOSED OR CLOSING when writing to the WS connection.

@mxschmitt mxschmitt requested a review from dgozman September 25, 2021 21:45
@mxschmitt mxschmitt added the CQ1 label Sep 25, 2021
Comment thread src/remote/playwrightClient.ts
@mxschmitt mxschmitt added CQ1 and removed CQ1 labels Sep 28, 2021
@mxschmitt mxschmitt changed the title test: unflake selectors-register.spec.ts - should work chore: fix PlaywrightClient disconnection logic Sep 28, 2021
@mxschmitt mxschmitt force-pushed the test/unflake-selectors-register branch from 7b62ff3 to cf1b4f3 Compare September 28, 2021 23:29
@mxschmitt mxschmitt added CQ1 and removed CQ1 labels Sep 28, 2021
@mxschmitt mxschmitt merged commit 55ddc55 into microsoft:master Sep 28, 2021
sidharthv96 added a commit to sidharthv96/playwright that referenced this pull request Oct 1, 2021
* upstream/master: (21 commits)
  fix(test runner): do not write missing snapshot until the last retry (microsoft#9246)
  feat(reporters): Add error position to JSON Report (microsoft#9151)
  feat(fetch): import/export storageState (microsoft#9244)
  fix: allow binary response interception (microsoft#9236)
  test(cookies): add a test for SameSite=None cookies (microsoft#9242)
  feat(webkit): roll WebKit to 1550 (microsoft#9239)
  test: add test for downloading PDF files (microsoft#9235)
  fix: leaking route handlers when times is used in Page.route (microsoft#9234)
  chore: upgrade commander.js to version 8 (microsoft#9230)
  feat(fetch): store cookies between requests (microsoft#9221)
  fix(selenium connect): register in gracefullyCloseAll for driver cleanup (microsoft#9218)
  fix(toBeHidden): return true to missing elements (microsoft#9205)
  chore: fix PlaywrightClient disconnection logic (microsoft#9149)
  fix(expect): beautiful expect stacks (microsoft#9204)
  feat(fetch): support ignoreHTTPSErrors option (microsoft#9206)
  feat(api): introduce locator.waitFor (microsoft#9200)
  feat(fetch): send Playwright as default user-agent for global fetch (microsoft#9195)
  feat(test runner): collect test error from worker teardown (microsoft#9190)
  test: add tests for Cross-Origin-Opener-Policy navigation (microsoft#9184)
  test: get response body for COOP responses (microsoft#9196)
  ...
sidharthv96 added a commit to sidharthv96/playwright that referenced this pull request Oct 1, 2021
…tionWithBaseChange

* upstream/master: (21 commits)
  fix(test runner): do not write missing snapshot until the last retry (microsoft#9246)
  feat(reporters): Add error position to JSON Report (microsoft#9151)
  feat(fetch): import/export storageState (microsoft#9244)
  fix: allow binary response interception (microsoft#9236)
  test(cookies): add a test for SameSite=None cookies (microsoft#9242)
  feat(webkit): roll WebKit to 1550 (microsoft#9239)
  test: add test for downloading PDF files (microsoft#9235)
  fix: leaking route handlers when times is used in Page.route (microsoft#9234)
  chore: upgrade commander.js to version 8 (microsoft#9230)
  feat(fetch): store cookies between requests (microsoft#9221)
  fix(selenium connect): register in gracefullyCloseAll for driver cleanup (microsoft#9218)
  fix(toBeHidden): return true to missing elements (microsoft#9205)
  chore: fix PlaywrightClient disconnection logic (microsoft#9149)
  fix(expect): beautiful expect stacks (microsoft#9204)
  feat(fetch): support ignoreHTTPSErrors option (microsoft#9206)
  feat(api): introduce locator.waitFor (microsoft#9200)
  feat(fetch): send Playwright as default user-agent for global fetch (microsoft#9195)
  feat(test runner): collect test error from worker teardown (microsoft#9190)
  test: add tests for Cross-Origin-Opener-Policy navigation (microsoft#9184)
  test: get response body for COOP responses (microsoft#9196)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants