Fix image upload crashes #76707
Merged
Merged
Conversation
Addresses console errors during AVIF processing by suppressing wasm-vips internal stdout/stderr output. Adds defensive guards against HEIC/HEIF input types that crash wasm-vips. Implements worker recycling after 50 image processing operations to prevent out-of-memory crashes from WASM memory growth. Adds performance benchmarking test for image upload processing times across formats and batch sizes. Fixes #76706. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @kleisauke. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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: +590 B (+0.01%) Total Size: 7.97 MB 📦 View Changed
ℹ️ View Unchanged
|
HEIC is already routed to server-side processing by prepareItem() since it's not in CLIENT_SIDE_SUPPORTED_MIME_TYPES. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… asset path Disable the libvips operation cache (Cache.max(0)) to prevent out-of-memory crashes during repeated image processing. The cache retains results from previous operations, accumulating WASM memory over time. Fix suggested by @kleisauke (wasm-vips maintainer). Also fix the E2E assets path in media processing performance tests which had one too many parent directory traversals, causing ENOENT errors when the tests run in the CI performance test environment. Props kleisauke. Fixes #76706 (issue 2).
The vips instance now disables the operation cache on init, but the test mock was missing the Cache property, causing a TypeError in CI.
|
Flaky tests detected in b8aee5c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26067542672
|
6 tasks
Addresses console errors during AVIF processing by suppressing wasm-vips internal stdout/stderr output. Adds defensive guards against HEIC/HEIF input types that crash wasm-vips. Implements worker recycling after 50 image processing operations to prevent out-of-memory crashes from WASM memory growth. Adds performance benchmarking test for image upload processing times across formats and batch sizes. Fixes #76706. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HEIC is already routed to server-side processing by prepareItem() since it's not in CLIENT_SIDE_SUPPORTED_MIME_TYPES. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… asset path Disable the libvips operation cache (Cache.max(0)) to prevent out-of-memory crashes during repeated image processing. The cache retains results from previous operations, accumulating WASM memory over time. Fix suggested by @kleisauke (wasm-vips maintainer). Also fix the E2E assets path in media processing performance tests which had one too many parent directory traversals, causing ENOENT errors when the tests run in the CI performance test environment. Props kleisauke. Fixes #76706 (issue 2).
The vips instance now disables the operation cache on init, but the test mock was missing the Cache property, causing a TypeError in CI.
When a child sideload item (e.g. thumbnail) is cancelled due to a vips error, the parent item was never notified to re-check its Finalize gate. This left the parent stuck forever with a spinner because processItem returned early at the hasPendingItemsByParentId check, and nobody called processItem(parentId) again after the children were removed. This mirrors the existing success path in processItem which already calls dispatch.processItem(parentId) when the last child completes normally.
When cancelItem removes an item that was actively doing ResizeCrop, Rotate, or Upload, the concurrency slot it held becomes available. But unlike finishOperation, cancelItem wasn't triggering pending items that were waiting for that slot. This caused remaining thumbnail children to sit in the queue forever, keeping the parent stuck at Finalize.
4b595c7 to
91e960e
Compare
adamsilverstein
commented
Apr 2, 2026
| @@ -0,0 +1,260 @@ | |||
| /** | |||
Member
Author
There was a problem hiding this comment.
@youknowriad I took another pass at e2e tests that actually upload images to time the "pipeline" an image must go thru. These don't seem to take as long as the previous version. I could reduce iterations or variations if needed, but as is I think this is taking just a few minutes.
adamsilverstein
commented
Apr 2, 2026
When vips handles image processing client-side, the server doesn't need to generate sub-sizes. Explicitly sending generate_sub_sizes: false tells the server to skip its image-type support check, which otherwise rejects formats like AVIF that the server's GD/Imagick can't process.
When all thumbnail children fail (e.g. unsupported AVIF profile), the parent item was completing successfully via Finalize, leaving the block showing an uploaded image that has no sub-sizes. Now when the last child is cancelled and no children remain, the parent is also cancelled so the block resets to its placeholder state.
Show 'This image cannot be processed. Convert it to JPEG or PNG before uploading.' instead of the generic 'File could not be uploaded' when all thumbnail children fail for an unsupported image format.
Contributor
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for the follow-ups here, and also thanks for the patience! I'm still working through a (very) long review backlog.
In principle I like a lot of the fixes here and in the UI this feels much nicer than getting stuck on an endless spinner:
2026-04-17.16.56.43.mp4
However, one problem I ran into with testing this is that in the above case it looks to the user as though the upload failed in a graceful and atomic way — the image block goes back to its placeholder state.
But in this case, if I go to the media library, I see that the AVIF file I attempted to upload has been uploaded, it's in the media library:
How should we handle that failure state? I imagine with the client-side processing architecture it's a tricky one to deal with because we're uploading the main image first before generating sub-sizes. Whereas as a user this looks to me like "the server rejected it so nothing was uploaded".
If that proves tricky to dig into, it might be worth splitting out the performance tests change from this PR into a separate PR, if it isn't too related to the upload crashes fixes?
Await vipsPromise locally so Cache.max(0) can run on the resolved instance. Adopt trunk's media-processing spec (#76792) and drop the PR's E2E upload benchmarks; reporter metric names now match trunk's mediaProcessingJpeg/Avif/JpegToAvif.
…/image-upload-crash Unify the HEIC/JPEG client conversion branch with the explicit generate_sub_sizes:false setting so vips-processed images skip the server image-type support check. Union the performance-reporter metric types so both trunk's mediaProcessing* and the PR's uploadProcessing benchmarks are present.
The IMAGE_PROCESSING_ERROR message surfaced when client-side sub-size processing fails is shown directly to the user, so it should be translatable. Other UploadError messages in this package are already translated; this one was missed when the parent-cancellation path was added.
When client-side sub-size processing fails (e.g. an unsupported AVIF profile that vips can't decode), the parent file has already been uploaded to the server and an attachment has been created. Previously we cancelled the parent item — resetting the block UI — but left the attachment behind, where it appeared in the media library as a ghost upload that looked successful from the user's perspective. Add a mediaDelete callback to the upload-media Settings interface, wired through editor/use-block-editor-settings to a thin apiFetch DELETE on /wp/v2/media/:id?force=true. cancelItem now invokes it as best-effort cleanup when cancelling a parent that has an attachment ID, mirroring the existing mediaFinalize wiring pattern.
Member
Author
|
Thanks @andrewserong — addressed the open feedback in the latest pushes:
Performance tests: I'd still like to keep them in this PR since they exercise the same end-to-end pipeline this fix targets, but happy to split them out if it's blocking review. |
The parent-cancel branch in cancelItem fires whenever the last in-flight child sideload fails — but the cause varies. vips processing failures (IMAGE_TRANSCODING_ERROR, IMAGE_ROTATION_ERROR) deserve the actionable 'convert to JPEG/PNG' hint, but a transient network failure or server-side validation rejection during sideload should surface its real message instead of misleading the user. Branch on error.code: keep the existing message for vips failures, otherwise propagate the underlying error.message (with a generic fallback). Preserve the original error code on the parent so callers can still distinguish failure types programmatically.
The parent-cancel branch in cancelItem fires when the last in-flight child sideload fails. Previously this branch unconditionally deleted the parent attachment and cancelled the parent — but 'no in-flight children remain' is not the same as 'no child succeeded': successful children are removed from the queue immediately, so 4-of-5 successes followed by a single failure still landed in this branch and threw away an attachment with 4 valid sub-sizes accumulated. Branch on parentItem.subSizes: if any sub-size was already accumulated, nudge the parent to advance past its Finalize gate and finalize with what we have (matching WP core's best-effort behavior when individual sub-size generation fails). Only fall through to the delete + cancel path when no child succeeded. Add four cancelItem tests covering the parent-cancel branch: vips-failure path, network-failure path with original message propagation, partial-success preservation, and the empty-message fallback.
4 tasks
andrewserong
approved these changes
Apr 29, 2026
Contributor
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for the follow-ups! The mediaDelete addition is consistent with the other methods we've exposed, and while it does add an extra one, I think it's important for the upload-media package to be responsible for its own cleanup, so it's a preferable way to do it rather than making consumers handle it, so this seems good to me.
In testing, uploading images and groups of images to gallery blocks is working as expected, and my avif file that should error out is erroring out as expected while not leaving any stray images in the file system or media library.
Left a couple of questions, but nothing blocking. When it comes to the performance tests, I don't mind their inclusion, but given there've been a few cases recently of the tests ballooning out in their runtime and blocking PR workflows, it'd be good to make sure they're as efficient as they can be. I left a comment about whether we could reduce the number of times we're creating a post for example, but this might be a bit nitpicky.
I reckon this is in pretty good shape, and overall a good idea to get in!
Comment on lines
+66
to
+74
| // Redirect wasm-vips internal stdout/stderr to prevent console errors | ||
| // (e.g. AVIF codec warnings that are not actionable for users). | ||
| // Set globalThis.__vipsDebug to a function to capture this output during development. | ||
| print: ( text: string ) => { | ||
| ( globalThis as any ).__vipsDebug?.( text ); | ||
| }, | ||
| printErr: ( text: string ) => { | ||
| ( globalThis as any ).__vipsDebug?.( text ); | ||
| }, |
Contributor
There was a problem hiding this comment.
I noticed that the error message is quite general now. Is that because of this change? Not a blocker, we can always tweak error messages separately.
Edit: I left a different comment, I think this change is unrelated. Feel free to ignore this one!
Member
Author
There was a problem hiding this comment.
Acknowledged and happy to ignore as you noted. The vips message wording wasn't changed in this PR; the differentiation work in cancelItem only chooses which existing message to show. Happy to revisit the wording in a follow-up if it reads as too general.
swissspidy
reviewed
Apr 29, 2026
Comment on lines
+236
to
+247
| // Total failure: no child succeeded. The parent file | ||
| // already uploaded — delete the orphaned attachment | ||
| // from the server so it doesn't appear in the media | ||
| // library. | ||
| const parentAttachmentId = parentItem.attachment?.id; | ||
| const { mediaDelete } = select.getSettings(); | ||
| if ( parentAttachmentId && mediaDelete ) { | ||
| mediaDelete( parentAttachmentId ).catch( () => { | ||
| // Best-effort cleanup; surface nothing to the | ||
| // user if the delete itself fails. | ||
| } ); | ||
| } |
Member
There was a problem hiding this comment.
When could this happen?
Is this a good user experience? If the failures aren't permanent, would it be worth retrying the processing?
Member
Author
There was a problem hiding this comment.
When this fires: parent file uploaded successfully, but every child sub-size sideload failed (vips couldn't decode any size, or every sideload network call failed). If even one sub-size succeeded we now take the partial-success branch instead and keep the parent attachment.
Is this a good UX: the user still gets an error notice; deleting the orphan keeps the media library clean. The silent .catch() on mediaDelete is best-effort — if it fails the orphan stays but the error notice has already surfaced, so we don't pile a second notice on top.
Retries: agreed retry would be the better UX, but it's intentionally out of scope here. Producer-side auto-retry with eventual-failure is being built as a follow-up in #76765; this PR's job is to stop the spinner from hanging forever when retries don't yet exist.
adamsilverstein
added a commit
that referenced
this pull request
Apr 29, 2026
These benchmarks belong with the throwaway-detection work being landed in #76707. Keep this PR focused on E2E coverage for the client-side media processing pipeline.
Each variant previously called createNewPost on every iteration, forcing ~39 full editor reloads across the spec. Move createNewPost outside the iteration loop, and reset state between iterations by removing the inserted block and clearing media. Single-image variants share a runUploadIterations helper to avoid duplication.
After an image upload completes, browser DOM focus stays inside the block (e.g., on figcaption), so `editor.selectBlocks` followed by pressing Backspace does not delete the block. The leftover block then causes a Playwright strict mode violation on the next iteration when two `Block: Image` figures are present. Replace the keyboard-based cleanup with a deterministic dispatch of `resetBlocks([])`, the same pattern used elsewhere in perf-utils.
adamsilverstein
added a commit
that referenced
this pull request
May 6, 2026
* Add test image assets for media processing E2E Adds image files covering format, transparency, EXIF rotation, animation, and oversized dimensions for use in client-side media processing E2E tests. Includes a Node.js generation script for reproducibility. * Add test plugins for image format conversion Plugins for E2E tests: PNG-to-JPEG conversion, JPEG-to-WebP conversion, and progressive/interlaced image saving via WordPress filters. * Add E2E tests for client-side media processing Covers upload, compression, sub-size generation, batch uploads, server fallback, error handling, browser capabilities, MIME format conversion, EXIF rotation, oversized scaling, and transparency preservation. * Add performance benchmarks for media upload Measures single image, large image with processing, and multiple simultaneous upload timings. Adds new metrics to the performance reporter. * Fix lint and TypeScript errors in E2E and performance tests - Remove unused `page` destructuring from test functions that access page via fixture - Fix prettier formatting for single-line locator and path.join calls - Add JSDoc type annotations to pushBits/bitsToBytes functions and bits variable - Add eslint-enable directive for playwright/expect-expect in media-upload perf test - Remove unused eslint-disable-next-line for playwright/no-skipped-test in fixture method Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix remaining lint and prettier formatting errors - Add eslint-disable no-bitwise for binary file generator script - Remove unused channels variable in createPNG - Fix prettier formatting in generate-test-media-assets.mjs - Fix JSDoc alignment, getSelectedBlockImageId formatting, and locator string formatting in client-side-media-processing E2E spec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix E2E test assertions for CI environment compatibility - Increase compression size tolerance from 3x to 5x for varying server configs - Accept both original and converted MIME types for format conversion tests - Accept both rotated and non-rotated dimensions for EXIF orientation test - Fix error handling test to check both snackbar and notice components Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix server upload failure test to assert on block state The test was waiting for a snackbar/error notice that never appears due to a double-unwrapping bug in the error callback chain between the editor and upload-media packages. Instead, assert that the upload queue drains and the image block remains in placeholder state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Skip flaky server upload failure test The test cannot reliably intercept uploads made from within the editor iframe's fetch context during client-side media processing. Additionally, the error callback chain has a double-unwrapping bug that prevents errors from surfacing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add missing responsive lightbox asset to generator The 3200x2400 responsive lightbox JPEG was committed as a test asset but was not included in the generator script. * Fix prettier formatting in generate-test-media-assets.mjs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Simplify E2E tests: reduce duplication and page loads Address reviewer feedback by consolidating redundant tests: - Merge duplicate JPEG tests (basic + MIME type + below threshold) into one - Merge duplicate PNG tests (basic + MIME type) into one - Merge 3 sub-size tests into one comprehensive test - Merge gallery and batch completion tests - Merge 3 browser capability tests into one - Remove duplicate transparency test from Special handling section - Remove unused image-interlaced-progressive.php plugin - Remove generate-test-media-assets.mjs generator script - Remove unused .webp and .avif test assets Reduces from ~21 active tests to 12, cutting page loads significantly. * Add WebP and AVIF upload tests for decode coverage Add E2E tests that upload WebP and AVIF images to exercise the wasm-vips decode path for these formats. Previously only JPEG, PNG, and GIF uploads were tested. Include valid static test assets generated from real pixel data. * E2E: Tighten client-side media processing tests to assert CSM-specific behavior Address review feedback on #75949: assertions were permissive enough to pass whether CSM ran or the server-side path took over. The image and gallery block e2es already cover the non-CSM path, so this spec should focus on behavior that is only true when the CSM pipeline runs. - Skip every test when CSM is not the active upload path (mirrors the `__clientSideMediaProcessing` + feature-detection gate used by the editor). - Drop the "fall back to server-side" test; that path is covered elsewhere and the assertions duplicated existing image-block e2es. - Drop the standalone "verify browser capabilities" test; the new skip helper enforces those preconditions for every test. - Replace mime-type unions (`[A, B].toContain`) with exact `toBe` assertions: WebP stays WebP, AVIF stays AVIF, and filtered conversions must produce the configured target. - Require thumbnail/medium/large sub-sizes (and assert the scaled main file's exact dimensions) instead of the previous "at least one size" check. - Require the rotated dimensions for the EXIF-rotated asset rather than accepting the unrotated original. - Extract the shared insert/upload/wait/fetch flow into a helper. * E2E: Remove media upload performance tests These benchmarks belong with the throwaway-detection work being landed in #76707. Keep this PR focused on E2E coverage for the client-side media processing pipeline. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
For unsupported sources (e.g. an AVIF profile wasm-vips can't decode), the parent's threshold-resize step throws before any child sideloads are queued. cancelItem then runs on the parent itself, so the parent-cancellation branch — which carried the 'web server cannot generate' wording — was bypassed and the user saw the raw 'File could not be uploaded' instead. Move the actionable wording to its source in resizeCropItem and rotateItem (where the vips failure is caught). Both paths now end up with the same user-facing message: parent failing on its own resize, or child sideload failing and propagating to parent. The vips-specific branch in cancelItem becomes redundant; collapse it to just propagate the underlying error's code and message.
Member
Author
|
Good catch @andrewserong — investigated and fixed in e119959. Root cause: Fix: Move the actionable wording to its source in The vips-specific message-rewriting branch in |
Move the vips operation counter and threshold from private-actions.ts into utils/index.ts (next to terminateVipsWorker) and expose a maybeRecycleVipsWorker(activeCount) helper. Call it from cancelItem in addition to finishOperation so that a long burst of vips failures (e.g. a gallery of unsupported AVIFs) can't bypass the recycle budget and grow WASM memory unbounded. Also drop the vipsModulePromise/vipsModule reset in terminateVipsWorker: worker recreation is driven by getWorkerAPI() inside @wordpress/vips/worker clearing its own workerAPI reference, not by re-importing the module (ES module imports are cached, so re-importing returns the same instance). The reset was a no-op and the comment claiming it was "essential" was misleading.
talldan
pushed a commit
that referenced
this pull request
May 8, 2026
* Add test image assets for media processing E2E Adds image files covering format, transparency, EXIF rotation, animation, and oversized dimensions for use in client-side media processing E2E tests. Includes a Node.js generation script for reproducibility. * Add test plugins for image format conversion Plugins for E2E tests: PNG-to-JPEG conversion, JPEG-to-WebP conversion, and progressive/interlaced image saving via WordPress filters. * Add E2E tests for client-side media processing Covers upload, compression, sub-size generation, batch uploads, server fallback, error handling, browser capabilities, MIME format conversion, EXIF rotation, oversized scaling, and transparency preservation. * Add performance benchmarks for media upload Measures single image, large image with processing, and multiple simultaneous upload timings. Adds new metrics to the performance reporter. * Fix lint and TypeScript errors in E2E and performance tests - Remove unused `page` destructuring from test functions that access page via fixture - Fix prettier formatting for single-line locator and path.join calls - Add JSDoc type annotations to pushBits/bitsToBytes functions and bits variable - Add eslint-enable directive for playwright/expect-expect in media-upload perf test - Remove unused eslint-disable-next-line for playwright/no-skipped-test in fixture method Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix remaining lint and prettier formatting errors - Add eslint-disable no-bitwise for binary file generator script - Remove unused channels variable in createPNG - Fix prettier formatting in generate-test-media-assets.mjs - Fix JSDoc alignment, getSelectedBlockImageId formatting, and locator string formatting in client-side-media-processing E2E spec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix E2E test assertions for CI environment compatibility - Increase compression size tolerance from 3x to 5x for varying server configs - Accept both original and converted MIME types for format conversion tests - Accept both rotated and non-rotated dimensions for EXIF orientation test - Fix error handling test to check both snackbar and notice components Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix server upload failure test to assert on block state The test was waiting for a snackbar/error notice that never appears due to a double-unwrapping bug in the error callback chain between the editor and upload-media packages. Instead, assert that the upload queue drains and the image block remains in placeholder state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Skip flaky server upload failure test The test cannot reliably intercept uploads made from within the editor iframe's fetch context during client-side media processing. Additionally, the error callback chain has a double-unwrapping bug that prevents errors from surfacing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add missing responsive lightbox asset to generator The 3200x2400 responsive lightbox JPEG was committed as a test asset but was not included in the generator script. * Fix prettier formatting in generate-test-media-assets.mjs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Simplify E2E tests: reduce duplication and page loads Address reviewer feedback by consolidating redundant tests: - Merge duplicate JPEG tests (basic + MIME type + below threshold) into one - Merge duplicate PNG tests (basic + MIME type) into one - Merge 3 sub-size tests into one comprehensive test - Merge gallery and batch completion tests - Merge 3 browser capability tests into one - Remove duplicate transparency test from Special handling section - Remove unused image-interlaced-progressive.php plugin - Remove generate-test-media-assets.mjs generator script - Remove unused .webp and .avif test assets Reduces from ~21 active tests to 12, cutting page loads significantly. * Add WebP and AVIF upload tests for decode coverage Add E2E tests that upload WebP and AVIF images to exercise the wasm-vips decode path for these formats. Previously only JPEG, PNG, and GIF uploads were tested. Include valid static test assets generated from real pixel data. * E2E: Tighten client-side media processing tests to assert CSM-specific behavior Address review feedback on #75949: assertions were permissive enough to pass whether CSM ran or the server-side path took over. The image and gallery block e2es already cover the non-CSM path, so this spec should focus on behavior that is only true when the CSM pipeline runs. - Skip every test when CSM is not the active upload path (mirrors the `__clientSideMediaProcessing` + feature-detection gate used by the editor). - Drop the "fall back to server-side" test; that path is covered elsewhere and the assertions duplicated existing image-block e2es. - Drop the standalone "verify browser capabilities" test; the new skip helper enforces those preconditions for every test. - Replace mime-type unions (`[A, B].toContain`) with exact `toBe` assertions: WebP stays WebP, AVIF stays AVIF, and filtered conversions must produce the configured target. - Require thumbnail/medium/large sub-sizes (and assert the scaled main file's exact dimensions) instead of the previous "at least one size" check. - Require the rotated dimensions for the EXIF-rotated asset rather than accepting the unrotated original. - Extract the shared insert/upload/wait/fetch flow into a helper. * E2E: Remove media upload performance tests These benchmarks belong with the throwaway-detection work being landed in #76707. Keep this PR focused on E2E coverage for the client-side media processing pipeline. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Resolve types.ts conflict by keeping both changes: trunk's widened mediaFinalize return type (srcset fix) and the new mediaDelete setting.
Closed
7 tasks
adamsilverstein
added a commit
that referenced
this pull request
May 21, 2026
…ision Reflect four client-side media PRs merged after these docs were last revised, keeping the architecture and how-to guides in sync with current behavior: - #76707: document the disabled libvips operation cache (Cache.max(0)) and the 50-operation worker recycling that bound WASM linear-memory growth, plus the globalThis.__vipsDebug hook for capturing the suppressed vips output. - #74903: document the sideload endpoint's image-dimension validation and its 400 error responses (rest_upload_dimension_mismatch and friends). - #78359: note that the finalize endpoint returns the refreshed attachment, which the editor uses to keep the block URL (and front-end srcset) in sync. - #78038: clarify that sub-size filenames derive from the original basename and only the scaled full-size copy carries the -scaled suffix.
| const path = require( 'path' ); | ||
| const fs = require( 'fs/promises' ); | ||
| const os = require( 'os' ); | ||
| const { v4: uuid } = require( 'uuid' ); |
Member
There was a problem hiding this comment.
We moved away from uuid to Node's built-in crypto.randomUUID() in #77960 because it causes issues with some builds when used in CJS modules because uuid dropped support for CJS in the latest versions and it's now ESM only.
Can we please do the same here?
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
4 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.
Summary
Addresses several issues reported in #76706 with client-side media processing:
Cache.max(0)to prevent libvips from caching results of previous operations, which caused unbounded WASM memory growth and OOM crashes.cancelItemnow kicks pending items waiting on concurrency slots freed by the cancelled item, so remaining children actually get processed instead of sitting in queue forever.cancelItemnow notifies the parent item to re-check its Finalize gate, so the parent completes once all children are done.globalThis.__vipsDebugfor debugging.media-upload.spec.jsmeasuring end-to-end image upload + processing times for JPEG, PNG, large JPEG, and batch uploads (5 images).Fixes #76706.
Test plan
npx wp-scripts test-unit-js --config test/unit/jest.config.js --testPathPattern='packages/(vips|upload-media)'npm run test:performance -- test/performance/specs/media-upload.spec.js(requires wp-env)