fix: handle page close during response event #398
Conversation
| @@ -43,7 +43,7 @@ describe('network', () => { | |||
| const driver = await Gatherer.setupDriver({ wsEndpoint }); | |||
| const network = new NetworkManager(driver); | |||
| await network.start(); | |||
| await driver.page.goto(server.TEST_PAGE, { waitUntil: 'networkidle' }); | |||
| await driver.page.goto(server.TEST_PAGE); | |||
There was a problem hiding this comment.
this is the actual test, without this change we will see unhandled error as page object would be closed before the response event fires.
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
| results: Array<NetworkInfo> = []; | ||
| _currentStep: Partial<Step> = null; | ||
|
|
||
| constructor(private driver: Driver) {} | ||
|
|
||
| private _addBarrier(page: Page, promise: Promise<void>) { |
There was a problem hiding this comment.
Are we trying to ensure that we collect header information before the page closes, or just prevent the unhandled exceptions in this case, or both?
Reading through the logic, it's clear to me how this would prevent unhandled exceptions from the page closing before the main promise promise is fulfilled. I'm having more trouble understanding the logic for preventing the page from closing before the data is collected, if that's part of this.
There was a problem hiding this comment.
Any async information that we are extracting from the response object or request object should happen during the page lifecycle,
if the page is closed, the associated information is lost and this barrier enforces that we capture the information on the network entry only during the existence of the page.
…a-fleet-heartbeat-synthetics-e2e-tests * upstream/master: [jjbb] for scheduling e2e jobs (elastic#418) docs: release notes for beta.17 (elastic#415) 1.0.0-beta.17 feat: add --playwright-options (elastic#405) fix: ensure timestamps of events are inorder (elastic#413) fix: do not stash `.nvm` folder (elastic#414) deprecate suite-params option over params (elastic#406) fix: end timestamp should reflect duration (elastic#410) docs: release notes for 1.16.0 (elastic#403) 1.0.0-beta.16 fix: revert Playwright to 1.14.0 (elastic#402) docs: release notes for beta 15 (elastic#400) 1.0.0-beta.15 chore: update deps and fix lint issues (elastic#399) fix: handle page close during response event (elastic#398) chore: require node > 14 for usage (elastic#397) fix: fix megabytes to mbps (elastic#394) feat: enable trace as part of rich-events (elastic#387) feat: use events from PW for network data (elastic#372)
Note: This technique is used by Hartracer in Playwright and we use the same here, we might move to PW HarTracer once its extendable.