-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Fix image upload crashes #76707
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
Merged
Merged
Fix image upload crashes #76707
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
a321f00
Fix image upload crashes and add media processing benchmarks
adamsilverstein ae6cdf5
Remove HEIC defensive guard from vips operations
adamsilverstein 1e916a5
Disable vips operation cache to prevent OOM crashes and fix perf test…
adamsilverstein b5fca7d
Add Cache mock to wasm-vips test fixture
adamsilverstein e35fd52
Fix image upload crashes and add media processing benchmarks
adamsilverstein 1568e7f
Remove HEIC defensive guard from vips operations
adamsilverstein 7b675be
Disable vips operation cache to prevent OOM crashes and fix perf test…
adamsilverstein 856327e
Add Cache mock to wasm-vips test fixture
adamsilverstein 0ae4690
Fix parent item stuck in spinner when child sideload fails
adamsilverstein 91e960e
Kick pending queue items when a cancelled item frees a concurrency slot
adamsilverstein 96bc77a
Send generate_sub_sizes: false for client-side processed images
adamsilverstein b1b37a7
Cancel parent upload when all child sideloads fail
adamsilverstein 354dbd9
Improve error message when image processing fails
adamsilverstein 6e16c0e
Match server-side error message for unsupported image types
adamsilverstein 23653fd
Suppress console errors for child sideload failures
adamsilverstein cc6a581
Add 1024x768 PNG test image for media upload benchmarks
adamsilverstein 930eee6
Merge branch 'trunk' into fix/image-upload-crash
adamsilverstein 619070e
Merge remote-tracking branch 'origin/fix/image-upload-crash' into fix…
adamsilverstein 04488db
Wrap user-facing image-upload error in __()
adamsilverstein 9339469
Delete orphaned attachment when client-side sub-size processing fails
adamsilverstein a29afa8
Differentiate parent-cancel error message by underlying cause
adamsilverstein 568b15b
Preserve parent attachment when partial sub-sizes succeeded
adamsilverstein c186044
Reuse single editor lifecycle in media upload perf tests
adamsilverstein 9441a0f
Fix flaky cleanup in media upload perf test
adamsilverstein e119959
Surface actionable error when parent vips processing fails
adamsilverstein 89a8c5b
Track failed vips ops toward worker recycle budget
adamsilverstein b8aee5c
Merge trunk into fix/image-upload-crash
adamsilverstein File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import apiFetch from '@wordpress/api-fetch'; | ||
|
|
||
| export default async function mediaDelete( id ) { | ||
| await apiFetch( { | ||
| path: `/wp/v2/media/${ id }?force=true`, | ||
| method: 'DELETE', | ||
| } ); | ||
| } |
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 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When could this happen?
Is this a good user experience? If the failures aren't permanent, would it be worth retrying the processing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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()onmediaDeleteis 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.