Map rs media upload failures to specific error types (cherry-pick #23023)#23028
Merged
adalpari merged 3 commits intoJun 24, 2026
Merged
Conversation
* Map rs media upload failures to specific error types The Application-Password REST upload path collapsed four distinct rs failures (HTTP status, WpError, execution failure, unknown) into a single GENERIC_ERROR, hiding the cause and showing users an unhelpful generic snackbar. Map by HTTP status and transport reason, and record status/code/reason in logMessage so reports are diagnosable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Satisfy detekt: extract error-parsing helpers and suppress LargeClass Split parseMediaError's network-error branches into small helper functions to stay under the LongMethod and CyclomaticComplexity thresholds, and mark the growing test class with @Suppress("LargeClass") to match the sibling TaxonomyRsApiRestClientTest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Extract shared buildMediaError helper to dedupe error parsers The four parse* helpers repeated the same MediaError.apply construction. Centralize it in buildMediaError, which also unifies the nullable statusCode handling. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Reuse MediaErrorType.fromHttpStatusCode and align 403 handling Replace the duplicated private HTTP-status mapper in MediaRSApiRestClient with the canonical MediaErrorType.fromHttpStatusCode, reconciling 403 to NOT_AUTHENTICATED (was AUTHORIZATION_REQUIRED) and restoring 400 -> BAD_REQUEST. Generalize fromHttpStatusCode's 500 case to the full 5xx range and make the execution-reason mapping exhaustive so new upstream RequestExecutionErrorReason variants surface as compile errors. Extend NOT_AUTHENTICATED handling through EditPostActivity, GutenbergKitActivity, and WPMediaUtils so 403s still show the no-permission message. Add tests for the 5xx range and forbidden execution reason. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/26.9 #23028 +/- ##
================================================
+ Coverage 37.54% 37.56% +0.01%
================================================
Files 2325 2325
Lines 125264 125324 +60
Branches 17173 17187 +14
================================================
+ Hits 47029 47073 +44
- Misses 74421 74433 +12
- Partials 3814 3818 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
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.
Description
Cherry-pick of #23023 (
8a595bb4f38) ontorelease/26.9. The cherry-pick applied cleanly with no conflicts.Self-hosted sites authenticated with an Application Password upload media through the new
wordpress-rsREST path (MediaRSApiRestClient). When an upload failed, four genuinely differentWpRequestResultoutcomes —InvalidHttpStatusCode,WpError,RequestExecutionFailed, andUnknownError— were all collapsed into a singleGENERIC_ERRORwith the message "Unknown error occurred". The real cause (HTTP status, error code, transport reason) only existed in a$mediaResponselog line, so user reports surfaced as an undiagnosable generic snackbar ("Couldn't complete this action").This PR splits that catch-all into specific handling:
InvalidHttpStatusCode/WpError/UnknownError) reuses the existingMediaErrorType.fromHttpStatusCode()rather than duplicating the logic:400 → BAD_REQUEST,401 → AUTHORIZATION_REQUIRED,403 → NOT_AUTHENTICATED,404 → NOT_FOUND,413 → REQUEST_TOO_LARGE,5xx → SERVER_ERROR, elseGENERIC_ERROR.fromHttpStatusCodewas also generalized so the whole5xxrange (not just500) maps toSERVER_ERROR.RequestExecutionFailed) viamediaErrorTypeFromExecutionReason(): timeout →TIMEOUT, offline/SSL →CONNECTION_ERROR, forbidden →NOT_AUTHENTICATED, auth required/rejected/misconfigured →AUTHORIZATION_REQUIRED, non-existent site →NOT_FOUND. Thewhenis intentionally exhaustive (noelse) so a new upstreamRequestExecutionErrorReasonvariant becomes a compile error here instead of silently degrading toGENERIC_ERROR.MediaError.statusCodeand a structuredlogMessage(status, error code, reason, request URL and method) so future reports are self-diagnosing.NOT_AUTHENTICATEDmapping for 403 is wired through the UI layers (EditPostActivity,GutenbergKitActivity,WPMediaUtils) so those failures still show the existing no-permission message.Because the upload snackbar text is driven by
MediaError.type, this also improves the user-facing message: many failures now show an actionable message (e.g. "media too large", "you don't have permission to upload") instead of the generic one.Testing instructions
Covered by unit tests in
MediaRsApiRestClientTestandMediaStoreTest. To verify locally:./gradlew :libs:fluxc:testDebugUnitTest --tests "*.MediaRsApiRestClientTest" --tests "*.MediaStoreTest"On-device sanity check (self-hosted site logged in with an Application Password):
🤖 Generated with Claude Code