Add comprehensive error handling and retry logic for upload-media#74917
Open
adamsilverstein wants to merge 25 commits intotrunkfrom
Open
Add comprehensive error handling and retry logic for upload-media#74917adamsilverstein wants to merge 25 commits intotrunkfrom
adamsilverstein wants to merge 25 commits intotrunkfrom
Conversation
|
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: +1.86 kB (+0.03%) Total Size: 6.87 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 340bec5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22346626200
|
adamsilverstein
force-pushed
the
74366-add-retry-and-error-handling
branch
from
22b6f13 to
34b509a
Compare
github-actions
bot
added
[Package] Block library
adamsilverstein
force-pushed
the
74366-add-retry-and-error-handling
branch
from
c5d67ea to
7d17226
Compare
github-actions
bot
removed
[Package] Core data
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive error handling and retry logic for the upload-media package to address GitHub Issue #74366. The implementation adds an error classification system with 16 error types, automatic retry with exponential backoff for transient failures, user-friendly localized error messages, and retry state management with a new ItemStatus.PendingRetry status.
Changes:
- Added
ErrorCodeenum with 16 error types categorized as retryable (network, timeout, server, VIPS worker errors) and non-retryable (validation, permission, file size, MIME type errors) - Implemented automatic retry with exponential backoff and jitter for failed uploads, with configurable retry settings (default: 3 attempts, 1s-30s delays, 2x multiplier)
- Created comprehensive error message system with localized, actionable user guidance for all error types
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/upload-media/src/upload-error.ts | Added ErrorCode enum and isRetryable getter to classify errors |
| packages/upload-media/src/error-messages.ts | New file providing user-friendly i18n error messages for all error codes |
| packages/upload-media/src/store/utils/retry.ts | New file with exponential backoff calculation and error classification logic |
| packages/upload-media/src/store/utils/debug-logger.ts | Added logging functions for retry scheduling, execution, and max retries exceeded |
| packages/upload-media/src/store/types.ts | Added RetrySettings, ScheduleRetryAction, ItemStatus.PendingRetry, and retry-related QueueItem fields |
| packages/upload-media/src/store/constants.ts | Added retry configuration constants (max attempts, delays, multiplier, jitter) |
| packages/upload-media/src/store/reducer.ts | Added ScheduleRetry case and default retry settings initialization |
| packages/upload-media/src/store/actions.ts | Modified cancelItem to check retry eligibility, added scheduleRetry and executeRetry actions |
| packages/upload-media/src/store/private-selectors.ts | Added selectors for retry state (pending items, retry timestamp, retry count, max retries check) |
| packages/upload-media/src/store/test/retry.ts | New test file with 28 unit tests for retry utilities |
| packages/upload-media/src/index.ts | Exported ErrorCode, error message functions, and retry-related types |
| packages/upload-media/README.md | Updated documentation for cancelItem, scheduleRetry, and executeRetry actions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5 tasks
adamsilverstein
force-pushed
the
feature/client-side-media-dev
branch
from
1fb78d7 to
89e2a02
Compare
adamsilverstein
force-pushed
the
74366-add-retry-and-error-handling
branch
2 times, most recently
from
9e4d2bc to
05918ca
Compare
adamsilverstein
added
[Type] Enhancement
adamsilverstein
changed the base branch from
feature/client-side-media-dev
to
trunk
Implements automatic retry with exponential backoff for failed uploads, error classification system, and user-friendly error messages. Changes: - Add ErrorCode enum with 16 error types (network, validation, processing) - Add isRetryable getter to UploadError class for automatic retry classification - Add retry configuration constants (max attempts, delays, backoff multiplier) - Add RetrySettings interface and ItemStatus.PendingRetry status - Add scheduleRetry/executeRetry actions for automatic retry scheduling - Add retry state selectors (getPendingRetryItems, getItemRetryCount, etc.) - Add retry logging functions for debugging - Add calculateRetryDelay utility with exponential backoff and jitter - Add shouldRetryError utility for error classification - Add user-friendly i18n error messages for all error codes - Add comprehensive unit tests for retry utilities (28 tests) The retry system automatically retries failed uploads for transient errors (network failures, timeouts, server errors) while immediately failing for non-retryable errors (validation, permissions, file size limits). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The nextRetryCount variable was only used for delay calculation and logging while currentRetryCount was passed to the action. Inline the +1 to reduce confusion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Return true when retry settings are not configured, since retries are effectively exceeded when they're disabled. Previously it defaulted to maxRetryAttempts: 3, returning a meaningful value even when retry was not configured. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add console.error mocking to tests that cancel items without an onError callback, preventing noisy test output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…anup Add test verifying a fresh AbortController is created when an item is retried, ensuring the signal is not already aborted. Add test verifying pending retry timers are cleared when an item is manually cancelled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove retryTimerId from ScheduleRetry test dispatches and replace the 'clears retryTimerId' test with a 'creates fresh AbortController' test to match the updated reducer behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The `code` property is typed as `string` for backward compatibility with existing callers that use string literals. The `includes()` call needs an explicit cast to satisfy TypeScript's strict type checking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
swissspidy
reviewed
Feb 25, 2026
swissspidy
reviewed
Feb 25, 2026
Address PR feedback: remove unused createTimer(), add measure() using the User Timings API (performance.measure) for DevTools Performance panel visibility under an "Upload Media" track. Add timing to upload, sideload, resize, rotate, and transcode operations. Add @param JSDoc tags to all exported functions and remove eslint-disable for jsdoc/require-param.
Extract clearRetryTimer helper from cancelItem and reuse it in removeItem so that removing an item during a pending retry properly cleans up the setTimeout reference from the module-level retryTimers Map. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-project-automation
bot
moved this to 🔎 Needs Review
in WordPress 7.0 Editor Tasks
swissspidy
reviewed
Feb 25, 2026
Moves the retry timer Map and cleanup helper out of actions.ts (public actions) into utils/retry.ts where other retry utilities live. This keeps clearRetryTimer out of the public store API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
Neat ideas here! Unfortunately I haven't been able to get it working for me locally for the case of a dropped network connection. When I reactivate the connection, the loaders still appear to be stuck:
I left a note about the logging, I'm wondering if that can be simplified / re-use warning rather than having lots of granular functions?
Without being able to test manually, the main question I had is what happens if you lose internet connection while uploading a large number of images to a gallery block at once. E.g. a 20-image gallery. Will it attempt to resume all the same time, or is it batched like for regular uploads? I'm also curious what the user expectation here is — how many retries / how long would we expect for it to attempt to upload versus cancelling out the operation 🤔
Separately, while attempting to test the case of an invalid file type (i.e. renaming a JPEG locally to use the .invalid extension) and then dragging and dropping to the editor, it appears to get stuck in a loading state in the canvas, with an error thrown:
I think in this case it's erroring here because there's no .url available on attachment (in this case attachment is undefined, I assume because the server returned a 500 error as the user was not allowed to upload the file type?):
I mightn't get a chance to re-review again before I wrap up for the week (I'm AFK tomorrow), but happy to test more next week if this is still open!
Member
Author
Ok let me work on testing this a bit more manually and try to get it working correctly. thanks for testing.
These should still get batched to avoid trying to do too much all at once.
Once the original image is uploaded we have everything we need to create all of the sub sizes. The expectation should be "when I upload my image to WordPress it will process that image to create all of the sub sizes used for my site. If my connection is interrupted, it will resume processing when the connection returns or the editor is reloaded" that woulkd also work if the user restarted their browser and reopened the post with the partially processed image. For genuine failures (for example lets say the sideload endpoint is failing), the system should fall back to using the existing server processing approach. so ideally users never notice the failure other than a slower than expected upload.
thanks for the debugging, i'll take a look. |
- Gut debug-logger: remove all log* functions, keep only measure() - Replace log* calls with @wordpress/warning in actions.ts - Fix attachment null crash in uploadItem onFileChange callback - Add missing network error retry patterns (fetch_error, Failed to fetch, Load failed) - Update tests for warning expectations and new retry patterns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The upload-media package imports @wordpress/warning but was missing the corresponding tsconfig project reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit added @wordpress/warning to upload-media's package.json but did not regenerate the lockfile, causing CI to fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
Testing this I noticed a couple of things are still off:
|
Guard uploadItem's finishOperation against double-fire from both onFileChange and onSuccess to prevent premature queue emptying and early save lock release. Add queueMissingSizeGeneration action and useMissingSizesCheck hook so the editor detects images with incomplete sub-sizes on load and queues client-side thumbnail generation.
Member
Author
|
Ready for more testing... |
Contributor
|
I just noticed that while in the site editor, this appears to hang for me in the loading state after selecting a file to upload: 
It doesn't happen for me in the post editor, but only in the site editor. Does that happen for you, or is it just me? |
Contributor
It turns out this is happening on trunk, too. I think I have a fix, I'll just test it a little more locally and then throw up a PR. |
Contributor
|
Fix in: #76124 |
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
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.
Fixes #74366
Summary
This PR implements comprehensive error handling and retry logic for the upload-media package, addressing GitHub Issue #74366.
Key Features:
ErrorCodeenum with 16 error types categorizing network errors, validation errors, processing errors, and user actionsItemStatus.PendingRetrystatus and selectors for tracking retry stateFiles Changed:
Modified:
packages/upload-media/src/upload-error.ts- Added ErrorCode enum and isRetryable getterpackages/upload-media/src/store/constants.ts- Added retry configuration constantspackages/upload-media/src/store/types.ts- Added RetrySettings, ScheduleRetryAction, extended QueueItempackages/upload-media/src/store/actions.ts- Added scheduleRetry, executeRetry, modified cancelItempackages/upload-media/src/store/reducer.ts- Added ScheduleRetry case and default retry settingspackages/upload-media/src/store/private-selectors.ts- Added retry state selectorspackages/upload-media/src/store/utils/debug-logger.ts- Added retry logging functionspackages/upload-media/src/index.ts- Exported new types and functionsCreated:
packages/upload-media/src/store/utils/retry.ts- Exponential backoff calculation and error classificationpackages/upload-media/src/error-messages.ts- User-friendly i18n error messagespackages/upload-media/src/store/test/retry.ts- Unit tests for retry utilities (28 tests)How It Works:
cancelItemchecks if the error is retryable usingshouldRetryError()scheduleRetry()calculates delay using exponential backoffPendingRetryand a timer schedulesexecuteRetry()Retry Configuration (defaults):
Test plan
🤖 Generated with Claude Code