Skip to content

fix: leaking route handlers when times is used in Page.route#9234

Merged
mxschmitt merged 1 commit into
microsoft:masterfrom
mxschmitt:bugfix/leaking-routes
Sep 30, 2021
Merged

fix: leaking route handlers when times is used in Page.route#9234
mxschmitt merged 1 commit into
microsoft:masterfrom
mxschmitt:bugfix/leaking-routes

Conversation

@mxschmitt
Copy link
Copy Markdown
Contributor

Fixes #9215

Comment thread src/client/page.ts Outdated
Comment thread src/client/network.ts Outdated
Comment thread src/client/browserContext.ts Outdated
@mxschmitt mxschmitt force-pushed the bugfix/leaking-routes branch from 9baf9e0 to 6911513 Compare September 30, 2021 10:16
@mxschmitt mxschmitt merged commit 7c89bc1 into microsoft:master Sep 30, 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)
  ...
// it can race with BrowserContext.close() which then throws since its closed
route.continue().catch(() => {});
} else {
this._routes = this._routes.filter(route => !route.expired());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mxschmitt Hi, I think setNetworkInterceptionEnabled enabled: false) can be called here when remaining handlers doesn't exist.

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] Clear finished Page.route({times}) handlers once they are done

3 participants