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

✨Cancelable @percy/core methods #654

Merged
merged 8 commits into from
Dec 13, 2021
Merged

✨Cancelable @percy/core methods #654

merged 8 commits into from
Dec 13, 2021

Conversation

wwilsman
Copy link
Contributor

@wwilsman wwilsman commented Dec 8, 2021

What is this?

Currently, tasks within the internal queue, including snapshots, are cancelable. This is done by utilizing generators as tasks. This PR takes that concept and uses it to make other core methods cancelable as well. Having these methods be cancelable will allow better handling of process termination in the CLI rather than relying on promises to completely settle.

While implementing this, I discovered that I had many hanging Chromium processes after several tests. This prompted me to tighten up some browser internals to alleviate it. Also while implementing this, the upload queue's concurrency was fixed to mirror the snapshot queue, and the setConfig method was updated to allow configuring the discovery concurrency after percy has already started.

To make certain core methods cancelable, the queue's cancelable generators were refactored into a common util and used to reimplement the waitFor helper. Since this helper is provided to the browser during execute, the new generatePromise helper must also be provided. This helper wraps an async generator function to become thennable and cancelable. This helper was then used in core methods such as start, stop, and others to allow them to be cancelable.

While making the stop method cancelable, part of it was refactored into a new flush method. In the future, this new method could also be used to flush snapshots from the queue when retries are enabled.

One caveat of the cancelable promise-like generator object is some implicit callback promise handling does not work as expected. In these situations, the returned object is awaited on directly to trigger running the underlying generator.

@wwilsman wwilsman added the ✨ enhancement New feature or request label Dec 8, 2021
@wwilsman wwilsman requested a review from Robdel12 December 8, 2021 22:15
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 Verrrrrry nice level up here

@wwilsman wwilsman merged commit dc076d8 into master Dec 13, 2021
@wwilsman wwilsman deleted the ww/cancelable-core branch December 13, 2021 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants