Client-side media: make batch thumbnail cancellation atomic#77977
Open
adamsilverstein wants to merge 1 commit intotrunkfrom
Open
Client-side media: make batch thumbnail cancellation atomic#77977adamsilverstein wants to merge 1 commit intotrunkfrom
adamsilverstein wants to merge 1 commit intotrunkfrom
Conversation
batchResizeImage() previously broke out of the resize loop on cancellation and returned whatever thumbnails it had produced so far, leaving callers to reconcile a partially populated result array against the requested size list. Switch to atomic semantics: when cancelOperations() fires mid-batch, throw a new OperationCancelledError and discard any already-generated thumbnails. Either every requested size is returned or the call rejects with no usable output. Document the contract on the function and export the error class so callers can distinguish user cancellation from processing failures. Resolves #77246
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +97 B (0%) Total Size: 7.94 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in cd0cb23. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25392895448
|
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
adamsilverstein
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Defines and implements the cancellation semantics for
batchResizeImage()inpackages/vips/src/index.ts, addressing the open question in #77246 (raised as a non-blocker on #76979).Why
batchResizeImage()previouslybreak'd the resize loop on mid-batch cancel and returned whatever thumbnails it had produced so far. That pushed the burden onto callers to figure out whether a result array was complete or truncated, and the function couldn't be safely composed with code that needs all-or-nothing guarantees.This change picks atomic semantics:
cancelOperations()fires mid-batch, the function throws a newOperationCancelledErrorand discards every already-generated thumbnail.Atomic was chosen because:
resizeImage,convertImageFormat, etc.) already propagate cancellation by killing the vips pipeline, which throws — atomic matches existing in-package behavior.The new
OperationCancelledErrorclass is exported so callers can distinguish user cancellation from real processing failures viainstanceofand suppress error reporting accordingly.Changes
OperationCancelledErrorclass inpackages/vips/src/index.ts(also re-exported asVipsOperationCancelledError).batchResizeImage()throws on mid-batch cancel instead of returning partial results.packages/vips/src/test/batch-resize-image.tscovering: clean batch path, cancel-mid-batch, and pre-cancel.packages/vips/README.md) regenerated.Testing instructions
npm run test:unit -- packages/vips/src/test/— 18 tests pass (3 new + 15 existing).npm run lint:js -- packages/vips/src/clean.Closes #77246