chore: make input actions "strict" in terms of timeout/abort #36302
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This introduces a "strict" mode to `Progress`, where it waits until the action completes instead of racing against a timeout. The action is responsible to terminate upon progress being aborted. To support this, some new APIs have been added to the progress: - `race()` to race any promise against the abort. Useful for things like `evaluate()` that do not change the state of the page, or single protocol commands. - `raceWithCleanup()` that combines `race()` and `cleanupWhenAborted()`. - `wait()` that waits for a timeout, but is aware of progress abort.
Contributor
Test results for "tests 1"4 flaky39367 passed, 820 skipped Merge workflow run. |
yury-s
reviewed
Jun 12, 2025
| }, | ||
| raceWithCleanup: <T>(promise: Promise<T>, cleanup: (result: T) => any) => { | ||
| return progress.race(promise.then(result => { | ||
| progress.cleanupWhenAborted(() => cleanup(result)); |
Member
There was a problem hiding this comment.
Why don't we run cleanup if the promise throws? Also, shouldn't cleanup be scheduled even if the promise is not finished?
Contributor
Author
There was a problem hiding this comment.
This is to allow the result escape the progress scope when the operation is successful. A typical example is newContext() that wants to return the context on success, but close it when the operation fails.
yury-s
approved these changes
Jun 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces a "strict" mode to
Progress, where it waits until the action completes instead of racing against a timeout.The action is responsible to terminate upon progress being aborted. The "rule of thumb" is - whenever a method accepts a progress, it must either succeed before the progress aborts or throw an error.
To support this new scheme, some new APIs have been added to the progress:
race()to race any promise against the abort. Useful for things likeevaluate()that do not change the state of the page, or single protocol commands. Many places now use this method to ensure they terminate when the progress is aborted.raceWithCleanup()that combinesrace()andcleanupWhenAborted().wait()that waits for a timeout, but is aware of progress abort.References #35987.