Use NetworkError for all API errors in the logic and application layers, regardless of login method #16035
Conversation
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
…perties - Add APIErrorDetails struct for parsing JSON error responses - Add convenience properties: isNotFound, isUnauthorized, isTimeout, isInvalidInput - Add apiErrorCode, apiErrorMessage, and userFriendlyMessage properties - Make response property public for broader access - Lay foundation for unified error handling across authentication methods 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
… DotcomErrors - Modify mapNetworkError to catch DotcomError and convert back to NetworkError - Preserve original HTTP status codes while adding parsed API error details - Add NetworkError.from(dotcomError:originalNetworkError:) conversion method - Use existing Constants and ErrorMessages from DotcomError instead of magic strings - Eliminates made-up status codes by preserving real ones from original NetworkError - All DotcomError-based requests now return NetworkError with real status codes This solves the core issue: DotcomErrors now maintain real HTTP status information while providing structured error details, enabling consistent error handling regardless of authentication method. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
… handling Key changes: - Replace DotcomError catch with NetworkError in synchronizeProducts method - Update ProductUpdateError.init to parse NetworkError.apiErrorCode instead of DotcomError - Update ProductLoadError.init to parse NetworkError.apiErrorCode instead of DotcomError - Replace remaining DotcomError.resourceDoesNotExist with NetworkError.invalidURL This completes the consolidation of error handling in ProductStore, eliminating authentication-dependent error inconsistencies. All product operations now return consistent NetworkError regardless of login method (application passwords vs other auth methods). Benefits: - Unified error handling across all authentication methods - Real HTTP status codes preserved from original network requests - Structured API error details available via NetworkError.apiErrorCode/Message - No more dual error handling paths for same error conditions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…of DotcomError - Replace DotcomError.unknown with NetworkError.unacceptableStatusCode in test mocks - Create proper JSON error response data for NetworkError parsing - Update test assertions to check NetworkError.apiErrorCode and apiErrorMessage - Maintains identical test logic while using consistent error handling approach These tests verify the product type filtering behavior where non-core types gracefully handle invalid parameter errors, while core types propagate the error. The behavior remains the same, only the error type representation has changed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…r consistently CouponsError.swift: - Simplify error handling to use NetworkError.apiErrorCode/Message directly - Remove unused ErrorDetails struct since NetworkError handles parsing OrderStore.swift: - Update gift card error handling to use NetworkError.apiErrorCode/Message - Maintain same error detection logic for gift card specific error codes PaymentsError.swift: - Consolidate DotcomError and NetworkError handling into single NetworkError path - Update PaymentsNetworkErrorDetails to use direct initialization - Remove unused PaymentsDotcomErrorDetails struct - Maintain same payment error code detection and conversion logic All files now use consistent NetworkError-based error handling while preserving existing error detection and user-facing error message functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…tore files ProductCategoryStore.swift: - Replace DotcomError.resourceDoesNotExist check with NetworkError.isNotFound ShippingLabelStore.swift: - Update REST route availability check to use NetworkError.apiErrorCode - Simplify PackageCreationError.init to use NetworkError.apiErrorCode directly SiteStore.swift: - Update SiteCreationError enum to store NetworkError instead of DotcomError - Convert DotcomError pattern matching to NetworkError.apiErrorCode checks - Maintain same error code detection for domain existence and validation StatsStoreV4.swift: - Update StatsError.init to map NetworkError codes to appropriate error cases - Map "unauthorized" → noPermission, "invalid_blog" → statsModuleDisabled, etc. WooShippingStore.swift: - Update REST route availability check to use NetworkError.apiErrorCode CommonReaderConfigProvider.swift: - Consolidate CardReaderConfigError.init to use NetworkError.apiErrorCode only - Remove duplicate handling of DotcomError and NetworkError cases - Maintain same error code detection for store address and postal code validation All Store files now use consistent NetworkError-based error handling with real HTTP status codes and structured API error details. This completes the unified error handling migration across the entire Yosemite layer. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
… layer Updated 14 files in the WooCommerce app layer to use NetworkError consistently: View Models & ViewControllers: - OrderDetailsViewModel: Update import to NetworkError - ConnectivityToolViewModel: Replace jetpackNotConnected check with apiErrorCode == "unknown_token" - EditableOrderViewModel: Update email/coupon error detection to use NetworkError.apiErrorCode - ErrorTopBannerFactory: Replace jetpackNotConnected check with apiErrorCode == "unknown_token" Dashboard Components (5 files): - ProductStockDashboardCardViewModel, MostActiveCouponsCardViewModel, TopPerformersDashboardViewModel, StorePerformanceViewModel: Replace DotcomError.noRestRoute checks with unified NetworkError pattern - DashboardViewModel: Remove unused DotcomError import Core Infrastructure: - DefaultStoresManager: Update REST route availability check for add-on groups - AuthenticatedState: Update comment to reference NetworkError instead of DotcomError - PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel: Update import to NetworkError All error detection logic preserved while using consistent NetworkError.apiErrorCode patterns. REST route availability, Jetpack connectivity, email validation, and coupon errors now work consistently across all authentication methods. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
joshheald
added
type: task
Generated by 🚫 Danger |
|
|
|
Version |
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.
Part of: WOOMOB-93
Description
There are two main error types that come out of the
Networkingframework.DotcomError, which is the main error for users logged in via Jetpack/WPcomNetworkError, for users logged in via application passwords.We have various application and logic layer code which handles specific errors. Usually, these only consider the
DotcomErrorvariant, so application password users don't get the specific error message or handling.This PR standardises on
NetworkError, so we don't have to double-handle in the application layers.Background
All
DotcomErrors start asNetworkErrors. TheDotcomValidatorconverts the errors whenRemote.mapNetworkErrorcalls it because the WPcom login route was used.That conversion removes some information (
statusCode) and adds much more (specific error messages, underlying error details.)All that information is usually present in a
NetworkErrorif it's come from an application password connection.How we fix it
The application shouldn't have to care how the user is logged in. We definitely shouldn't have to implement special case error handling twice for every error.
This PR standardises on NetworkError, because it can express everything.
We do this by preventing
DotcomErrorfrom being passed out of Networking, instead wrapping it back in aNetworkErroronce the extra detail has been unwrapped.Alternative approaches and improvements
I also tried wrapping both in a
WooAPIError. I found this was a much bigger change; there are a lot of tests based around NetworkError, so it's better to settle on that.I'd still like to improve the handling of codes in the application layer. For example, in
WooShippingStore, we look specifically forrest_no_route. This will continue to work, but will still only come from aDotcomError. We'd need to update to useerror.isNotFoundinstead, but in this case we may want to specifically handle the Jetpack tunnel saying that there's no route to this route.It would also be good to rely less on magic strings, especially in the application layer. We can do this more by moving to the new methods on NetworkError, but we may also just need to define our magic strings in a constants enum – some are specific to a single endpoint.
Steps to reproduce
Repeat this with the other login method.
Testing information
This PR is draft and shared for discussion. There are quite a few other specific error flows that I have yet to test!
Screenshots
RELEASE-NOTES.txtif necessary.