core(user-flow): support aborting pending navigations#17067
core(user-flow): support aborting pending navigations#17067ChristopherPHolder wants to merge 7 commits into
Conversation
connorjclark
left a comment
There was a problem hiding this comment.
looks promising!
could you also add something to docs/user-flows.md (perhaps under tips and tricks) about dispose()?
| }; | ||
| }); | ||
|
|
||
| /** @type {(event: Event) => void} */ |
There was a problem hiding this comment.
should check / use signal.throwIfAborted() too (source: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal#implementing_an_abortable_api)
There was a problem hiding this comment.
Nice was not aware of that API.
I implemented the changes.
But I am not super sure it make sense tho, if a user calls dispose the navigation is will simply cancel the wait for conditions.
A user is not able to call startNavigation if the last navigation was not cleanedup successfully. That throws a clear error.
On a seperate note, i have phased another issue in the past where the audit took extremely long because the page was polling. So the network and cpu were never really quiet.
In theory i can dispose of the await conditions when i see the element i want render on the page.
Kind of an abuse of the api tho...
Those things are edge cases and probably better to move forward and i open another issue if it becomes an issue again.
d684e5c to
84d43ca
Compare
84d43ca to
cf1b49a
Compare
- Fix an unhandled promise rejection in `core/user-flow.js` when a user flow navigation is aborted before completion by properly catching and ignoring the error if the controller's signal was aborted. - Ensure `gotoURL` completely respects the `AbortSignal` so it does not hang indefinitely when waiting for navigations or load events to resolve mid-flight. - Fix missing `throwIfAborted` method in the mock `AbortSignal` which caused the new tests in `wait-for-condition-test.js` to fail. - Remove an unused `EventEmitter` import in `mock-driver.js` that caused `npm run lint` to fail. - Expand the `.dispose()` documentation in `user-flows.md` to include a concrete javascript code example as requested by reviewers. - Add additional test coverage in `navigation-test.js` to verify the abort controller behavior.
|
@ChristopherPHolder I made a couple additions on top of your changes. If you have any thoughts on the additions let me know; otherwise I'll merge this for the next release. Thanks for your help! |
LGTM and thanks for your help and letting me contribute |
Summary
Introduces a way to gracefully dispose of ongoing wait for conditions in Userflow navigation audits.
This allows programatic usage to handle failed navigations without having to await for wait for conditions to throw timeout errors.
It also automatically handles disposal of the conditions when the puppeteer page is closed.
Example usage:
Related Issues/PRs
CLOSES: #17065