From 07d35d2516e65a4b386b936e93cc2bcd49dfb92b Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 15:58:25 +0100 Subject: [PATCH 01/19] Enhance NetworkError with better response parsing and convenience properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../NetworkingCore/Network/NetworkError.swift | 88 ++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/Modules/Sources/NetworkingCore/Network/NetworkError.swift b/Modules/Sources/NetworkingCore/Network/NetworkError.swift index c7aa44a270e..241624e442b 100644 --- a/Modules/Sources/NetworkingCore/Network/NetworkError.swift +++ b/Modules/Sources/NetworkingCore/Network/NetworkError.swift @@ -37,7 +37,7 @@ public enum NetworkError: Error, Equatable { } /// Response data accompanied the error if available - var response: Data? { + public var response: Data? { switch self { case .notFound(let response): return response @@ -49,6 +49,22 @@ public enum NetworkError: Error, Equatable { return nil } } + + /// Parsed API error details from response data + public var apiErrorDetails: APIErrorDetails? { + guard let data = response else { return nil } + return try? JSONDecoder().decode(APIErrorDetails.self, from: data) + } + + /// API error code from response, if available + public var apiErrorCode: String? { + return apiErrorDetails?.code + } + + /// API error message from response, if available + public var apiErrorMessage: String? { + return apiErrorDetails?.message + } } @@ -121,3 +137,73 @@ extension NetworkError: CustomStringConvertible { } } } + +// MARK: - Convenience Properties for Common Error Conditions + +public extension NetworkError { + /// Returns true if this error represents a "not found" condition + var isNotFound: Bool { + switch self { + case .notFound: + return true + case .unacceptableStatusCode(let statusCode, _): + return statusCode == 404 + default: + return false + } + } + + /// Returns true if this error represents an authorization issue + var isUnauthorized: Bool { + switch self { + case .invalidCookieNonce: + return true + case .unacceptableStatusCode(let statusCode, _): + return statusCode == 401 || statusCode == 403 + default: + // Check for specific unauthorized error codes in API response + return apiErrorCode?.contains("unauthorized") == true || + apiErrorCode?.contains("invalid_token") == true + } + } + + /// Returns true if this error represents a timeout + var isTimeout: Bool { + switch self { + case .timeout: + return true + case .unacceptableStatusCode(let statusCode, _): + return statusCode == 408 + default: + return false + } + } + + /// Returns true if this error represents invalid input/parameters + var isInvalidInput: Bool { + switch self { + case .unacceptableStatusCode(let statusCode, _): + return statusCode == 400 + default: + return apiErrorCode?.contains("invalid_param") == true + } + } + + /// Returns a user-friendly error message, preferring API error message over generic description + var userFriendlyMessage: String { + return apiErrorMessage ?? localizedDescription + } +} + +// MARK: - Supporting Types + +/// Represents error details from API response JSON +public struct APIErrorDetails: Codable { + public let code: String + public let message: String? + + public init(code: String, message: String?) { + self.code = code + self.message = message + } +} From 29a205bb346b3201467351b989b614b54a1a1f15 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 16:22:19 +0100 Subject: [PATCH 02/19] Update Remote.mapNetworkError to preserve real HTTP status codes from DotcomErrors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../NetworkingCore/Model/DotcomError.swift | 70 +++++++++++++++++++ .../NetworkingCore/Remote/Remote.swift | 5 ++ 2 files changed, 75 insertions(+) diff --git a/Modules/Sources/NetworkingCore/Model/DotcomError.swift b/Modules/Sources/NetworkingCore/Model/DotcomError.swift index 198492f9f43..e589cec2bd4 100644 --- a/Modules/Sources/NetworkingCore/Model/DotcomError.swift +++ b/Modules/Sources/NetworkingCore/Model/DotcomError.swift @@ -106,6 +106,10 @@ public enum DotcomError: Error, Decodable, Equatable, GeneratedFakeable { static let noStatsPermission = "user cannot view stats" static let resourceDoesNotExist = "Resource does not exist." static let jetpackNotConnected = "This blog does not have Jetpack connected" + static let unauthorized = "Missing or invalid authorization" + static let invalidToken = "Invalid authentication token" + static let requestFailed = "Request failed" + static let noRestRoute = "No matching REST route" } } @@ -163,3 +167,69 @@ public func ==(lhs: DotcomError, rhs: DotcomError) -> Bool { return false } } + +// MARK: - NetworkError Conversion +// +public extension NetworkError { + /// Creates a NetworkError from a DotcomError, preserving the original NetworkError's status code and response data + /// This is used in Remote.mapNetworkError to maintain real HTTP status codes while adding parsed API error details + static func from(dotcomError: DotcomError, originalNetworkError: NetworkError) -> NetworkError { + let enhancedErrorData = dotcomError.createEnhancedErrorResponseData(originalResponse: originalNetworkError.response) + + switch originalNetworkError { + case .notFound: + return .notFound(response: enhancedErrorData) + case .timeout: + return .timeout(response: enhancedErrorData) + case let .unacceptableStatusCode(statusCode, _): + return .unacceptableStatusCode(statusCode: statusCode, response: enhancedErrorData) + case .invalidURL: + return .invalidURL + case .invalidCookieNonce: + return .invalidCookieNonce + } + } +} + +// MARK: - Helper for NetworkError creation +// +private extension DotcomError { + /// Creates enhanced JSON error response data that preserves original response while adding structured error details + func createEnhancedErrorResponseData(originalResponse: Data?) -> Data? { + let (code, message) = getCodeAndMessage() + + let errorResponse: [String: Any] = [ + "code": code, + "message": message + ] + + // Return the enhanced structured error data (the original response is already parsed) + return try? JSONSerialization.data(withJSONObject: errorResponse) + } + + /// Extracts the appropriate error code and message for each DotcomError case + private func getCodeAndMessage() -> (code: String, message: String) { + switch self { + case .empty: + return ("empty", "Empty response") + case .unauthorized: + return (Constants.unauthorized, ErrorMessages.unauthorized) + case .noStatsPermission: + return (Constants.unauthorized, ErrorMessages.noStatsPermission) + case .invalidToken: + return (Constants.invalidToken, ErrorMessages.invalidToken) + case .requestFailed: + return (Constants.requestFailed, ErrorMessages.requestFailed) + case .noRestRoute: + return (Constants.noRestRoute, ErrorMessages.noRestRoute) + case .jetpackNotConnected: + return (Constants.unknownToken, ErrorMessages.jetpackNotConnected) + case .statsModuleDisabled: + return (Constants.invalidBlog, ErrorMessages.statsModuleDisabled) + case .resourceDoesNotExist: + return (Constants.restTermInvalid, ErrorMessages.resourceDoesNotExist) + case .unknown(let code, let message): + return (code, message ?? "Unknown error") + } + } +} diff --git a/Modules/Sources/NetworkingCore/Remote/Remote.swift b/Modules/Sources/NetworkingCore/Remote/Remote.swift index 98d0fcf8cad..891b965e7e6 100644 --- a/Modules/Sources/NetworkingCore/Remote/Remote.swift +++ b/Modules/Sources/NetworkingCore/Remote/Remote.swift @@ -295,6 +295,7 @@ private extension Remote { } /// Maps an error from `network.responseData` so that the request's corresponding error can be returned. + /// Returns a NetworkError with preserved status codes and enhanced API error details. /// func mapNetworkError(error: Error, for request: Request) -> Error { guard let networkError = error as? NetworkError else { @@ -317,7 +318,11 @@ private extension Remote { let validator = request.responseDataValidator() try validator.validate(data: response) return networkError + } catch let dotcomError as DotcomError { + // Convert DotcomError back to NetworkError while preserving original status code + return NetworkError.from(dotcomError: dotcomError, originalNetworkError: networkError) } catch { + // For non-DotcomError validation failures, return the original error return error } } From b7584b1fd8ca6dcfffb2f130197be35bd304c424 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 16:37:39 +0100 Subject: [PATCH 03/19] Update ProductStore to use NetworkError consistently across all error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Yosemite/Stores/ProductStore.swift | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/Modules/Sources/Yosemite/Stores/ProductStore.swift b/Modules/Sources/Yosemite/Stores/ProductStore.swift index 1a73dba5a2e..6600fb8c10d 100644 --- a/Modules/Sources/Yosemite/Stores/ProductStore.swift +++ b/Modules/Sources/Yosemite/Stores/ProductStore.swift @@ -310,7 +310,8 @@ private extension ProductStore { shouldDeleteExistingProducts: shouldDeleteExistingProducts) let hasNextPage = products.count == pageSize return hasNextPage - } catch let error as DotcomError where error == .unknown(code: "rest_invalid_param", message: "Invalid parameter(s): type") { + } catch let error as NetworkError where error.apiErrorCode == "rest_invalid_param" && + error.apiErrorMessage?.starts(with: "Invalid parameter(s): type") == true { if let productType, ProductType.coreTypes.contains(productType) == false { return false @@ -678,7 +679,7 @@ private extension ProductStore { feature: .productDetailsFromScannedTexts, responseFormat: .json) guard let jsonData = jsonString.data(using: .utf8) else { - return completion(.failure(DotcomError.resourceDoesNotExist)) + return completion(.failure(NetworkError.invalidURL)) } let details = try JSONDecoder().decode(ProductDetailsFromScannedTexts.self, from: jsonData) completion(.success(.init(name: details.name, description: details.description))) @@ -1376,19 +1377,15 @@ public enum ProductUpdateError: Error, Equatable { case generic(message: String) init(error: Error) { - guard let dotcomError = error as? DotcomError else { - self = .unknown(error: error.toAnyError) - return - } - switch dotcomError { - case let .unknown(code, message): - guard let errorCode = ErrorCode(rawValue: code) else { - self = .unknown(error: dotcomError.toAnyError) + if let networkError = error as? NetworkError, + let apiErrorCode = networkError.apiErrorCode { + guard let errorCode = ErrorCode(rawValue: apiErrorCode) else { + self = .unknown(error: error.toAnyError) return } - self = errorCode.error(with: message) - default: - self = .unknown(error: dotcomError.toAnyError) + self = errorCode.error(with: networkError.apiErrorMessage) + } else { + self = .unknown(error: error.toAnyError) } } @@ -1462,12 +1459,12 @@ public enum ProductLoadError: Error, Equatable { case unknown(error: AnyError) init(underlyingError error: Error) { - guard case let DotcomError.unknown(code, _) = error else { + if let networkError = error as? NetworkError, + let apiErrorCode = networkError.apiErrorCode { + self = ErrorCode(rawValue: apiErrorCode)?.error ?? .unknown(error: error.toAnyError) + } else { self = .unknown(error: error.toAnyError) - return } - - self = ErrorCode(rawValue: code)?.error ?? .unknown(error: error.toAnyError) } enum ErrorCode: String { From b0467f21a7776dac786435e71e704b80b16a3166 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 16:56:21 +0100 Subject: [PATCH 04/19] Update ProductStore FilterProducts tests to use NetworkError instead of DotcomError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../Stores/ProductStore+FilterProductsTests.swift | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Modules/Tests/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift b/Modules/Tests/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift index 608007b038e..00ecba11d21 100644 --- a/Modules/Tests/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift @@ -201,9 +201,9 @@ final class ProductStore_FilterProductsTests: XCTestCase { func test_synchronizeProducts_with_non_core_product_type_network_error_then_it_returns_success_without_next_page() throws { // Given let remote = MockProductsRemote() + let errorData = "{\"code\":\"rest_invalid_param\",\"message\":\"Invalid parameter(s): type\"}".data(using: .utf8)! remote.whenLoadingAllProducts(siteID: sampleSiteID, - thenReturn: .failure(DotcomError.unknown(code: "rest_invalid_param", - message: "Invalid parameter(s): type"))) + thenReturn: .failure(NetworkError.unacceptableStatusCode(statusCode: 400, response: errorData))) let productStore = ProductStore(dispatcher: dispatcher, storageManager: storageManager, network: network, @@ -236,9 +236,9 @@ final class ProductStore_FilterProductsTests: XCTestCase { func test_synchronizeProducts_with_core_product_type_network_error_then_it_returns_failure() throws { // Given let remote = MockProductsRemote() + let errorData = "{\"code\":\"rest_invalid_param\",\"message\":\"Invalid parameter(s): type\"}".data(using: .utf8)! remote.whenLoadingAllProducts(siteID: sampleSiteID, - thenReturn: .failure(DotcomError.unknown(code: "rest_invalid_param", - message: "Invalid parameter(s): type"))) + thenReturn: .failure(NetworkError.unacceptableStatusCode(statusCode: 400, response: errorData))) let productStore = ProductStore(dispatcher: dispatcher, storageManager: storageManager, network: network, @@ -264,8 +264,9 @@ final class ProductStore_FilterProductsTests: XCTestCase { // Then XCTAssertTrue(result.isFailure) let error = try XCTUnwrap(result.failure) - XCTAssertEqual(error as? DotcomError, .unknown(code: "rest_invalid_param", - message: "Invalid parameter(s): type")) + let networkError = error as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "rest_invalid_param") + XCTAssertEqual(networkError?.apiErrorMessage, "Invalid parameter(s): type") } } From da9775e9f73b7ff78dfbabca07bfa9340b20e351 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 16:56:31 +0100 Subject: [PATCH 05/19] Fix whitespace issues --- .../NetworkingCore/Model/DotcomError.swift | 8 ++--- .../NetworkingCore/Network/NetworkError.swift | 36 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Modules/Sources/NetworkingCore/Model/DotcomError.swift b/Modules/Sources/NetworkingCore/Model/DotcomError.swift index e589cec2bd4..d15fa6fe899 100644 --- a/Modules/Sources/NetworkingCore/Model/DotcomError.swift +++ b/Modules/Sources/NetworkingCore/Model/DotcomError.swift @@ -175,7 +175,7 @@ public extension NetworkError { /// This is used in Remote.mapNetworkError to maintain real HTTP status codes while adding parsed API error details static func from(dotcomError: DotcomError, originalNetworkError: NetworkError) -> NetworkError { let enhancedErrorData = dotcomError.createEnhancedErrorResponseData(originalResponse: originalNetworkError.response) - + switch originalNetworkError { case .notFound: return .notFound(response: enhancedErrorData) @@ -197,16 +197,16 @@ private extension DotcomError { /// Creates enhanced JSON error response data that preserves original response while adding structured error details func createEnhancedErrorResponseData(originalResponse: Data?) -> Data? { let (code, message) = getCodeAndMessage() - + let errorResponse: [String: Any] = [ "code": code, "message": message ] - + // Return the enhanced structured error data (the original response is already parsed) return try? JSONSerialization.data(withJSONObject: errorResponse) } - + /// Extracts the appropriate error code and message for each DotcomError case private func getCodeAndMessage() -> (code: String, message: String) { switch self { diff --git a/Modules/Sources/NetworkingCore/Network/NetworkError.swift b/Modules/Sources/NetworkingCore/Network/NetworkError.swift index 241624e442b..22176267a27 100644 --- a/Modules/Sources/NetworkingCore/Network/NetworkError.swift +++ b/Modules/Sources/NetworkingCore/Network/NetworkError.swift @@ -25,14 +25,14 @@ public enum NetworkError: Error, Equatable { /// The HTTP response code of the network error, for cases that are deducted from the status code. public var responseCode: Int? { switch self { - case .notFound: - return StatusCode.notFound - case .timeout: - return StatusCode.timeout - case let .unacceptableStatusCode(statusCode, _): - return statusCode - default: - return nil + case .notFound: + return StatusCode.notFound + case .timeout: + return StatusCode.timeout + case let .unacceptableStatusCode(statusCode, _): + return statusCode + default: + return nil } } @@ -49,19 +49,19 @@ public enum NetworkError: Error, Equatable { return nil } } - + /// Parsed API error details from response data public var apiErrorDetails: APIErrorDetails? { guard let data = response else { return nil } return try? JSONDecoder().decode(APIErrorDetails.self, from: data) } - + /// API error code from response, if available public var apiErrorCode: String? { return apiErrorDetails?.code } - - /// API error message from response, if available + + /// API error message from response, if available public var apiErrorMessage: String? { return apiErrorDetails?.message } @@ -152,7 +152,7 @@ public extension NetworkError { return false } } - + /// Returns true if this error represents an authorization issue var isUnauthorized: Bool { switch self { @@ -163,10 +163,10 @@ public extension NetworkError { default: // Check for specific unauthorized error codes in API response return apiErrorCode?.contains("unauthorized") == true || - apiErrorCode?.contains("invalid_token") == true + apiErrorCode?.contains("invalid_token") == true } } - + /// Returns true if this error represents a timeout var isTimeout: Bool { switch self { @@ -178,7 +178,7 @@ public extension NetworkError { return false } } - + /// Returns true if this error represents invalid input/parameters var isInvalidInput: Bool { switch self { @@ -188,7 +188,7 @@ public extension NetworkError { return apiErrorCode?.contains("invalid_param") == true } } - + /// Returns a user-friendly error message, preferring API error message over generic description var userFriendlyMessage: String { return apiErrorMessage ?? localizedDescription @@ -201,7 +201,7 @@ public extension NetworkError { public struct APIErrorDetails: Codable { public let code: String public let message: String? - + public init(code: String, message: String?) { self.code = code self.message = message From c70df8efeb236ac3c3e46f2e6786acc65c4ad986 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 17:14:56 +0100 Subject: [PATCH 06/19] Update CouponsError, OrderStore, and PaymentsError to use NetworkError consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Yosemite/Stores/CouponsError.swift | 21 ++--------- .../Sources/Yosemite/Stores/OrderStore.swift | 5 +-- .../Yosemite/Stores/PaymentsError.swift | 36 +++++-------------- .../Remote/JetpackSettingsRemoteTests.swift | 3 +- .../NetworkingTests/Remote/RemoteTests.swift | 15 ++++---- .../POSProductOrVariationResolverTests.swift | 7 ++-- .../WooShippingCreateLabelsViewModel.swift | 4 +-- .../ProductDownloadListViewController.swift | 4 +-- 8 files changed, 33 insertions(+), 62 deletions(-) diff --git a/Modules/Sources/Yosemite/Stores/CouponsError.swift b/Modules/Sources/Yosemite/Stores/CouponsError.swift index 19fdb92f106..d6470d843b7 100644 --- a/Modules/Sources/Yosemite/Stores/CouponsError.swift +++ b/Modules/Sources/Yosemite/Stores/CouponsError.swift @@ -8,17 +8,11 @@ public struct CouponsError: Error, LocalizedError { public init?(underlyingError error: Error) { switch error { - case DotcomError.unknown(Constants.invalidCouponCode, let message): - self.message = message ?? Localizations.defaultCouponsError - self.underlyingError = error - case let NetworkError.unacceptableStatusCode(_, response): - guard let response, - let errorDetails = try? JSONDecoder().decode(ErrorDetails.self, from: response), - errorDetails.code == Constants.invalidCouponCode - else { + case let networkError as NetworkError: + guard networkError.apiErrorCode == Constants.invalidCouponCode else { return nil } - self.message = errorDetails.message ?? Localizations.defaultCouponsError + self.message = networkError.apiErrorMessage ?? Localizations.defaultCouponsError self.underlyingError = error default: return nil @@ -31,15 +25,6 @@ public struct CouponsError: Error, LocalizedError { } - private struct ErrorDetails: Decodable { - let code: String - let message: String? - - enum CodingKeys: CodingKey { - case code - case message - } - } enum Localizations { static let defaultCouponsError = NSLocalizedString( diff --git a/Modules/Sources/Yosemite/Stores/OrderStore.swift b/Modules/Sources/Yosemite/Stores/OrderStore.swift index a4093adbfaa..58d1907bd50 100644 --- a/Modules/Sources/Yosemite/Stores/OrderStore.swift +++ b/Modules/Sources/Yosemite/Stores/OrderStore.swift @@ -662,8 +662,9 @@ private extension OrderStore { onCompletion(result) }) case .failure(let error): - if let dotcomError = error as? DotcomError, - case let .unknown(code, message) = dotcomError { + if let networkError = error as? NetworkError, + let code = networkError.apiErrorCode, + let message = networkError.apiErrorMessage { switch code { case "woocommerce_rest_gift_card_cannot_apply": return onCompletion(.failure(GiftCardError.cannotApply(reason: message))) diff --git a/Modules/Sources/Yosemite/Stores/PaymentsError.swift b/Modules/Sources/Yosemite/Stores/PaymentsError.swift index 8d3fe9ed975..0b8362f1ab9 100644 --- a/Modules/Sources/Yosemite/Stores/PaymentsError.swift +++ b/Modules/Sources/Yosemite/Stores/PaymentsError.swift @@ -12,53 +12,35 @@ public enum PaymentsError: Error, LocalizedError { return } - /// See if we recognize this DotcomError code + /// See if we recognize this NetworkError code /// self = errorDetails.asPaymentsError() ?? .otherError(error: error.toAnyError) } private static func unwrapError(error: Error) -> PaymentsErrorConvertible? { switch error { - case let DotcomError.unknown(code, message): - return PaymentsDotcomErrorDetails(code: code, message: message) - case let NetworkError.unacceptableStatusCode(_, response): - guard let response, - let errorDetails = try? JSONDecoder().decode(PaymentsNetworkErrorDetails.self, from: response) else { + case let networkError as NetworkError: + guard let code = networkError.apiErrorCode, + let errorCode = PaymentsErrorCode(rawValue: code) else { return nil } - return errorDetails + return PaymentsNetworkErrorDetails(code: errorCode, message: networkError.apiErrorMessage) default: return nil } } - private struct PaymentsNetworkErrorDetails: Decodable, PaymentsErrorConvertible { + private struct PaymentsNetworkErrorDetails: PaymentsErrorConvertible { let code: PaymentsErrorCode let message: String? - enum CodingKeys: CodingKey { - case code - case message - } - } - - private struct PaymentsDotcomErrorDetails: PaymentsErrorConvertible { - // The response JSON differs from NetworkError above: - // `"error": "wcpay_get_charge", "message": "Error fetching charge:"` - // It's also decoded further up the chain. - let code: PaymentsErrorCode - let message: String? - - init?(code: String, message: String?) { - guard let errorCode = PaymentsErrorCode(rawValue: code) else { - return nil - } - - self.code = errorCode + init(code: PaymentsErrorCode, message: String?) { + self.code = code self.message = message } } + public var errorDescription: String? { switch self { case .noSuchChargeError(let message): diff --git a/Modules/Tests/NetworkingTests/Remote/JetpackSettingsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/JetpackSettingsRemoteTests.swift index 7df97e4e287..bdb0c16e98b 100644 --- a/Modules/Tests/NetworkingTests/Remote/JetpackSettingsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/JetpackSettingsRemoteTests.swift @@ -31,7 +31,8 @@ final class JetpackSettingsRemoteTests: XCTestCase { // When try await remote.enableJetpackModule(for: sampleSiteID, moduleSlug: "invalidmodule") }, errorAssert: { error in - (error as? DotcomError) == .unknown(code: "some_updated", message: "Invalid option: invalidmodule.") + let networkError = error as? NetworkError + return networkError?.apiErrorCode == "some_updated" && networkError?.apiErrorMessage == "Invalid option: invalidmodule." }) } diff --git a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift index ad2bca7ac67..af0a08379aa 100644 --- a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift @@ -180,8 +180,8 @@ final class RemoteTests: XCTestCase { do { let _: String = try await remote.enqueue(request) } catch { - let error = try XCTUnwrap(error as? DotcomError) - XCTAssertEqual(error, .requestFailed) + let networkError = try XCTUnwrap(error as? NetworkError) + XCTAssertEqual(networkError.apiErrorCode, "http_request_failed") } await fulfillment(of: [expectationForNotification], timeout: Constants.expectationTimeout) @@ -202,7 +202,7 @@ final class RemoteTests: XCTestCase { remote.enqueue(request, mapper: mapper) { (payload, error) in XCTAssertNil(payload) - XCTAssert(error is DotcomError) + XCTAssert(error is NetworkError) expectationForRequest.fulfill() } @@ -234,7 +234,7 @@ final class RemoteTests: XCTestCase { // Then XCTAssertTrue(try XCTUnwrap(result).isFailure) - XCTAssertTrue(try XCTUnwrap(result?.failure) is DotcomError) + XCTAssertTrue(try XCTUnwrap(result?.failure) is NetworkError) } /// Verifies that `enqueuePublisher` posts a `RemoteDidReceiveJetpackTimeoutError` Notification whenever the backend returns a Request Timeout error. @@ -258,7 +258,8 @@ final class RemoteTests: XCTestCase { // Then XCTAssertTrue(result.isFailure) - XCTAssertEqual(result.failure as? DotcomError, DotcomError.requestFailed) + let networkError = result.failure as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "http_request_failed") } /// Verifies that dotcom v1.1 request parses DotcomError @@ -954,7 +955,7 @@ final class RemoteTests: XCTestCase { /// Verifies that `enqueue:mapper:` maps an error from `responseData` when error has proper response data /// - func test_enqueue_request_throws_DotcomError_from_NetworkError_with_proper_response_data() throws { + func test_enqueue_request_throws_NetworkError_from_NetworkError_with_proper_response_data() throws { // Given let network = MockNetwork() let mapper = DummyMapper() @@ -980,7 +981,7 @@ final class RemoteTests: XCTestCase { // Then XCTAssertNil(result.0) XCTAssertNotNil(result.1) - XCTAssertTrue(result.1 is DotcomError) + XCTAssertTrue(result.1 is NetworkError) } } diff --git a/Modules/Tests/YosemiteTests/PointOfSale/POSProductOrVariationResolverTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/POSProductOrVariationResolverTests.swift index f8c1223f848..a5007b9d419 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/POSProductOrVariationResolverTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/POSProductOrVariationResolverTests.swift @@ -164,10 +164,11 @@ struct POSProductOrVariationResolverTests { ) let scannedCode = "test-barcode-loading" let someError = NSError(domain: "Test", code: 1, userInfo: nil) - let dotcomNotFoundError = DotcomError.unknown(code: "woocommerce_rest_product_invalid_id", message: "Not found") + let errorData = "{\"code\":\"woocommerce_rest_product_invalid_id\",\"message\":\"Not found\"}".data(using: .utf8)! + let networkNotFoundError = NetworkError.unacceptableStatusCode(statusCode: 404, response: errorData) - // NotFound case (simulate DotcomError for not found) - mockProductsRemote.whenLoadingProductForPointOfSale(siteID: variation.siteID, productID: variation.parentID, thenReturn: .failure(dotcomNotFoundError)) + // NotFound case (simulate NetworkError for not found) + mockProductsRemote.whenLoadingProductForPointOfSale(siteID: variation.siteID, productID: variation.parentID, thenReturn: .failure(networkNotFoundError)) await #expect(throws: PointOfSaleBarcodeScanError.noParentProductForVariation(scannedCode: scannedCode)) { _ = try await sut.itemForProductOrVariation(variation, scannedCode: scannedCode) } diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift index 2c4f19b240c..8122b040974 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShippingCreateLabelsViewModel.swift @@ -3,7 +3,7 @@ import Yosemite import WooFoundation import Combine import struct Networking.WooShippingAccountSettings -import enum Networking.DotcomError +import enum Networking.NetworkError import protocol Storage.StorageManagerType enum WooShippingCreateLabelSelection { @@ -373,7 +373,7 @@ final class WooShippingCreateLabelsViewModel: ObservableObject { } let markOrderComplete = isOrderCompleted ? nil : self.markOrderComplete try await currentShipmentDetailsViewModel.purchaseLabel(markOrderComplete: markOrderComplete) - } catch let DotcomError.unknown(code, _) where code == Constants.missingUPSDAPTermsOfServiceAcceptance { + } catch let error as NetworkError where error.apiErrorCode == Constants.missingUPSDAPTermsOfServiceAcceptance { shouldShowUPSTermsAndConditions = true } catch { labelPurchaseErrorNotice = Notice(title: Localization.LabelPurchaseError.title, diff --git a/WooCommerce/Classes/ViewRelated/Products/Edit Product/Downloadable Files/File List/ProductDownloadListViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Edit Product/Downloadable Files/File List/ProductDownloadListViewController.swift index 62162e4e304..0b05218b146 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Edit Product/Downloadable Files/File List/ProductDownloadListViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Products/Edit Product/Downloadable Files/File List/ProductDownloadListViewController.swift @@ -3,7 +3,7 @@ import UIKit import Yosemite import Photos import MobileCoreServices -import enum Networking.DotcomError +import enum Networking.NetworkError final class ProductDownloadListViewController: UIViewController { private let product: ProductFormDataModel @@ -348,7 +348,7 @@ private extension ProductDownloadListViewController { func showMediaUploadAlert(error: Error) { let errorMessage: String = { switch error { - case DotcomError.unknown(let code, _) where code == Constants.unsupportedMimeTypeCode: + case let error as NetworkError where error.apiErrorCode == Constants.unsupportedMimeTypeCode: Localization.unsupportedFileType case MediaAssetExporter.AssetExportError.unsupportedPHAssetMediaType: Localization.unsupportedFileType From 2112515c5e1e129f4f80eb3c57853c29173d580f Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 17:19:07 +0100 Subject: [PATCH 07/19] Complete DotcomError to NetworkError migration across all remaining Store files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Stores/ProductCategoryStore.swift | 4 ++-- .../Yosemite/Stores/ShippingLabelStore.swift | 20 +++++++------------ .../Sources/Yosemite/Stores/SiteStore.swift | 15 +++++++------- .../Yosemite/Stores/StatsStoreV4.swift | 14 ++++++++----- .../Yosemite/Stores/WooShippingStore.swift | 3 ++- .../Tools/CommonReaderConfigProvider.swift | 19 ++++++++---------- 6 files changed, 35 insertions(+), 40 deletions(-) diff --git a/Modules/Sources/Yosemite/Stores/ProductCategoryStore.swift b/Modules/Sources/Yosemite/Stores/ProductCategoryStore.swift index a11899ddb90..bf27e0fc086 100644 --- a/Modules/Sources/Yosemite/Stores/ProductCategoryStore.swift +++ b/Modules/Sources/Yosemite/Stores/ProductCategoryStore.swift @@ -149,8 +149,8 @@ private extension ProductCategoryStore { onCompletion(.success(productCategory)) } case .failure(let error): - if let error = error as? DotcomError, - case .resourceDoesNotExist = error { + if let networkError = error as? NetworkError, + networkError.isNotFound { onCompletion(.failure(ProductCategoryActionError.categoryDoesNotExistRemotely)) } else { onCompletion(.failure(error)) diff --git a/Modules/Sources/Yosemite/Stores/ShippingLabelStore.swift b/Modules/Sources/Yosemite/Stores/ShippingLabelStore.swift index 62b9de9f7dd..4efa76ac8ad 100644 --- a/Modules/Sources/Yosemite/Stores/ShippingLabelStore.swift +++ b/Modules/Sources/Yosemite/Stores/ShippingLabelStore.swift @@ -161,7 +161,8 @@ private extension ShippingLabelStore { } onCompletion(eligibility.isEligible) case .failure(let error): - if error as? DotcomError == .noRestRoute { + if let networkError = error as? NetworkError, + networkError.apiErrorCode == "rest_no_route" { DDLogError("⚠️ Endpoint for shipping label creation eligibility is unreachable for order: \(orderID). WC Shipping plugin may be missing.") } else { DDLogError("⛔️ Error checking shipping label creation eligibility for order \(orderID): \(error)") @@ -487,19 +488,12 @@ public enum PackageCreationError: Error, Equatable { case unknown(error: AnyError) init(error: Error) { - guard let dotcomError = error as? DotcomError else { - self = .unknown(error: error.toAnyError) - return - } - switch dotcomError { - case .unknown(let code, _): - guard let errorCode = ErrorCode(rawValue: code) else { - self = .unknown(error: dotcomError.toAnyError) - return - } + if let networkError = error as? NetworkError, + let code = networkError.apiErrorCode, + let errorCode = ErrorCode(rawValue: code) { self = errorCode.error - default: - self = .unknown(error: dotcomError.toAnyError) + } else { + self = .unknown(error: error.toAnyError) } } diff --git a/Modules/Sources/Yosemite/Stores/SiteStore.swift b/Modules/Sources/Yosemite/Stores/SiteStore.swift index 3cd68049900..bf06b85433d 100644 --- a/Modules/Sources/Yosemite/Stores/SiteStore.swift +++ b/Modules/Sources/Yosemite/Stores/SiteStore.swift @@ -197,8 +197,8 @@ public enum SiteCreationError: Error, Equatable { /// When the site creation result is returned but its `success` boolean is `false`. case unsuccessful /// Unexpected error from WPCOM. - case unexpected(error: DotcomError) - /// Unknown error that is not a `DotcomError` nor `Networking.SiteCreationError`. + case unexpected(error: NetworkError) + /// Unknown error that is not a `NetworkError` nor `Networking.SiteCreationError`. case unknown(description: String) public init(remoteError: Error) { @@ -208,19 +208,18 @@ public enum SiteCreationError: Error, Equatable { case .invalidDomain: self = .invalidDomain } - case let remoteError as DotcomError: - switch remoteError { - case let .unknown(code, _): + case let networkError as NetworkError: + if let code = networkError.apiErrorCode { switch code { case "blog_name_exists": self = .domainExists case "blog_name_only_lowercase_letters_and_numbers": self = .invalidDomain default: - self = .unexpected(error: remoteError) + self = .unexpected(error: networkError) } - default: - self = .unexpected(error: remoteError) + } else { + self = .unexpected(error: networkError) } default: self = .unknown(description: remoteError.localizedDescription) diff --git a/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift b/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift index dab6219a254..2b615c630e8 100644 --- a/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift +++ b/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift @@ -628,16 +628,20 @@ public enum SiteStatsStoreError: Error { case unknown init(error: Error) { - guard let dotcomError = error as? DotcomError else { + guard let networkError = error as? NetworkError, + let code = networkError.apiErrorCode else { self = .unknown return } - switch dotcomError { - case .noStatsPermission: + switch code { + case "unauthorized": + // Maps to the old .noStatsPermission case self = .noPermission - case .statsModuleDisabled: + case "invalid_blog": + // Maps to the old .statsModuleDisabled case self = .statsModuleDisabled - case .jetpackNotConnected: + case "unknown_token": + // Maps to the old .jetpackNotConnected case self = .jetpackNotConnected default: self = .unknown diff --git a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift index bfef74b6df0..670fec064d2 100644 --- a/Modules/Sources/Yosemite/Stores/WooShippingStore.swift +++ b/Modules/Sources/Yosemite/Stores/WooShippingStore.swift @@ -120,7 +120,8 @@ private extension WooShippingStore { } onCompletion(eligibility.isEligible) case .failure(let error): - if error as? DotcomError == .noRestRoute { + if let networkError = error as? NetworkError, + networkError.apiErrorCode == "rest_no_route" { DDLogError("⚠️ Endpoint for shipping label creation eligibility is unreachable for order: \(orderID). WC Shipping plugin may be missing.") } else { DDLogError("⛔️ Error checking shipping label creation eligibility for order \(orderID): \(error)") diff --git a/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift b/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift index e65162b626a..5c1cc407fef 100644 --- a/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift +++ b/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift @@ -66,20 +66,17 @@ final public class CommonReaderConfigProvider: CommonReaderConfigProviding { private extension CardReaderConfigError { init?(error: Error) { switch error { - case DotcomError.unknown(code: "store_address_is_incomplete", let message): - self = .incompleteStoreAddress(adminUrl: URL(string: message ?? "")) - case DotcomError.unknown(code: "postal_code_invalid", _): - self = .invalidPostalCode - case NetworkError.unacceptableStatusCode(_, let responseData): - guard let responseData, - let details = try? JSONDecoder().decode(CardReaderConfigNetworkErrorDetails.self, from: responseData) else { + case let networkError as NetworkError: + guard let code = networkError.apiErrorCode else { return nil } - switch details.code { - case .storeAddressIncomplete: - self = .incompleteStoreAddress(adminUrl: URL(string: details.message ?? "")) - case .postalCodeInvalid: + switch code { + case "store_address_is_incomplete": + self = .incompleteStoreAddress(adminUrl: URL(string: networkError.apiErrorMessage ?? "")) + case "postal_code_invalid": self = .invalidPostalCode + default: + return nil } default: return nil From b2124e933101e13a86a2ae639f202c407d8a4114 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 17:25:28 +0100 Subject: [PATCH 08/19] Complete DotcomError to NetworkError migration across WooCommerce app layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...ymentValidatingOrderErrorMessageViewModel.swift | 2 +- .../Order Details/OrderDetailsViewModel.swift | 2 +- .../ConnectivityToolViewModel.swift | 2 +- .../Coupons/MostActiveCouponsCardViewModel.swift | 3 +-- .../ViewRelated/Dashboard/DashboardViewModel.swift | 1 - .../ProductStockDashboardCardViewModel.swift | 3 +-- .../StoreStats/StorePerformanceViewModel.swift | 3 +-- .../TopPerformersDashboardViewModel.swift | 3 +-- .../Order Creation/EditableOrderViewModel.swift | 14 +++++++------- .../Top Banner/ErrorTopBannerFactory.swift | 4 ++-- .../Classes/Yosemite/AuthenticatedState.swift | 2 +- .../Classes/Yosemite/DefaultStoresManager.swift | 4 ++-- 12 files changed, 19 insertions(+), 24 deletions(-) diff --git a/WooCommerce/Classes/POS/Presentation/Card Present Payments/Reader Messages/PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift b/WooCommerce/Classes/POS/Presentation/Card Present Payments/Reader Messages/PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift index 7fdef4c6634..93671725ba1 100644 --- a/WooCommerce/Classes/POS/Presentation/Card Present Payments/Reader Messages/PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift +++ b/WooCommerce/Classes/POS/Presentation/Card Present Payments/Reader Messages/PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift @@ -1,5 +1,5 @@ import Foundation -import enum Networking.DotcomError +import enum Networking.NetworkError struct PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel: Equatable { let title: String diff --git a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift index 67aa5a2f7ee..3e8011accfd 100644 --- a/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift @@ -7,7 +7,7 @@ import Combine import Experiments import WooFoundation import SwiftUI -import enum Networking.DotcomError +import enum Networking.NetworkError import protocol Storage.StorageManagerType final class OrderDetailsViewModel { diff --git a/WooCommerce/Classes/ViewRelated/Connectivity Tool/ConnectivityToolViewModel.swift b/WooCommerce/Classes/ViewRelated/Connectivity Tool/ConnectivityToolViewModel.swift index b89b7a8a790..69507ff4b2e 100644 --- a/WooCommerce/Classes/ViewRelated/Connectivity Tool/ConnectivityToolViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Connectivity Tool/ConnectivityToolViewModel.swift @@ -215,7 +215,7 @@ final class ConnectivityToolViewModel { "Read more about it or contact our support team and we will happily assist you.", comment: "Message when we there is a decoding error in the recovery tool") readMoreAction = .init(title: readMore, systemImage: SystemImages.readMore.rawValue, action: generalTroubleshootAction) - case (DotcomError.jetpackNotConnected, _): + case (let networkError as NetworkError, _) where networkError.apiErrorCode == "unknown_token": message = NSLocalizedString("There is problem with your jetpack connection.\n\n" + "Read more about it or contact our support team and we will happily assist you.", comment: "Message when we there is a jetpack error in the recovery tool") diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Coupons/MostActiveCouponsCardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Coupons/MostActiveCouponsCardViewModel.swift index 05253041baa..8e0a8192ef6 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Coupons/MostActiveCouponsCardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Coupons/MostActiveCouponsCardViewModel.swift @@ -2,7 +2,6 @@ import Foundation import Yosemite import protocol WooFoundation.Analytics import protocol Storage.StorageManagerType -import enum Networking.DotcomError import enum Networking.NetworkError /// View model for `MostActiveCouponsCard`. @@ -97,7 +96,7 @@ final class MostActiveCouponsCardViewModel: ObservableObject { analytics.track(event: .DynamicDashboard.cardLoadingCompleted(type: .coupons)) } catch { switch error { - case DotcomError.noRestRoute, NetworkError.notFound: + case let networkError as NetworkError where networkError.isNotFound || networkError.apiErrorCode == "rest_no_route": analyticsEnabled = false default: analyticsEnabled = true diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift index df7015b71f7..412c148f23d 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift @@ -1,7 +1,6 @@ import Foundation import Yosemite import Combine -import enum Networking.DotcomError import enum Storage.StatsVersion import protocol Storage.StorageManagerType import protocol Experiments.FeatureFlagService diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCardViewModel.swift index e675a5721ff..ef981942507 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/ProductStock/ProductStockDashboardCardViewModel.swift @@ -1,7 +1,6 @@ import Foundation import Yosemite import protocol WooFoundation.Analytics -import enum Networking.DotcomError import enum Networking.NetworkError /// View model for `ProductStockDashboardCard` @@ -53,7 +52,7 @@ final class ProductStockDashboardCardViewModel: ObservableObject { analytics.track(event: .DynamicDashboard.cardLoadingCompleted(type: .stock)) } catch { switch error { - case DotcomError.noRestRoute, NetworkError.notFound: + case let networkError as NetworkError where networkError.isNotFound || networkError.apiErrorCode == "rest_no_route": analyticsEnabled = false default: analyticsEnabled = true diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/StoreStats/StorePerformanceViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/StoreStats/StorePerformanceViewModel.swift index 0007b6773a6..cf40ca3cfdd 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/StoreStats/StorePerformanceViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/StoreStats/StorePerformanceViewModel.swift @@ -5,7 +5,6 @@ import WooFoundation import Yosemite import protocol Storage.StorageManagerType import enum Storage.StatsVersion -import enum Networking.DotcomError import enum Networking.NetworkError /// Different display modes of site visit stats @@ -191,7 +190,7 @@ final class StorePerformanceViewModel: ObservableObject { syncingDidFinishPublisher.send(nil) } catch { switch error { - case DotcomError.noRestRoute, NetworkError.notFound: + case let networkError as NetworkError where networkError.isNotFound || networkError.apiErrorCode == "rest_no_route": analyticsEnabled = false default: analyticsEnabled = true diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardViewModel.swift index 06cb12b8535..fb8f9b389aa 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/TopPerformers/TopPerformersDashboardViewModel.swift @@ -3,7 +3,6 @@ import Foundation import WooFoundation import Yosemite import protocol Storage.StorageManagerType -import enum Networking.DotcomError import enum Networking.NetworkError /// View model for `TopPerformersDashboardView` @@ -127,7 +126,7 @@ final class TopPerformersDashboardViewModel: ObservableObject { syncingDidFinishPublisher.send(nil) } catch { switch error { - case DotcomError.noRestRoute, NetworkError.notFound: + case let networkError as NetworkError where networkError.isNotFound || networkError.apiErrorCode == "rest_no_route": analyticsEnabled = false default: analyticsEnabled = true diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift index d94a43626e0..cf6dcf1973b 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/EditableOrderViewModel.swift @@ -4,7 +4,7 @@ import protocol Storage.StorageManagerType import Experiments import UIKit import WooFoundation -import enum Networking.DotcomError +import enum Networking.NetworkError /// Encapsulates the item type an order can have, products or variations /// @@ -2509,19 +2509,19 @@ extension EditableOrderViewModel { /// This is needed because old stores error when sending empty emails. /// private static func isEmailError(_ error: Error, order: Order) -> Bool { - switch error as? DotcomError { - case .unknown(code: "rest_invalid_param", let message?): + if let networkError = error as? NetworkError, + networkError.apiErrorCode == "rest_invalid_param", + let message = networkError.apiErrorMessage { return message.contains("billing") && order.billingAddress?.hasEmailAddress == false - default: - return false } + return false } private static func isCouponsError(_ error: Error) -> Bool { - if case .unknown(code: "woocommerce_rest_invalid_coupon", _) = error as? DotcomError { + if let networkError = error as? NetworkError, + networkError.apiErrorCode == "woocommerce_rest_invalid_coupon" { return true } - return false } diff --git a/WooCommerce/Classes/ViewRelated/Top Banner/ErrorTopBannerFactory.swift b/WooCommerce/Classes/ViewRelated/Top Banner/ErrorTopBannerFactory.swift index fa38c0de9a2..af7b58e8041 100644 --- a/WooCommerce/Classes/ViewRelated/Top Banner/ErrorTopBannerFactory.swift +++ b/WooCommerce/Classes/ViewRelated/Top Banner/ErrorTopBannerFactory.swift @@ -1,6 +1,6 @@ import Foundation import Yosemite -import enum Networking.DotcomError +import enum Networking.NetworkError /// Creates a top banner to be used when there is an error loading data on a screen. /// The banner has two action buttons: "Troubleshoot" and "Contact Support." @@ -90,7 +90,7 @@ extension ErrorTopBannerFactory { if error is DecodingError { self = .decodingError - } else if error as? DotcomError == .jetpackNotConnected { + } else if let networkError = error as? NetworkError, networkError.apiErrorCode == "unknown_token" { self = .jetpackConnectionError } else { self = .generalError diff --git a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift index ed1e45a7a2a..ed97918131b 100644 --- a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift +++ b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift @@ -178,7 +178,7 @@ private extension AuthenticatedState { } } - /// Executed whenever a DotcomError is received (ApplicationLayer). This allows us to have a *main* error handling flow! + /// Executed whenever a NetworkError is received (ApplicationLayer). This allows us to have a *main* error handling flow! /// func tunnelTimeoutWasReceived(note: Notification) { ServiceLocator.analytics.track(.jetpackTunnelTimeout) diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index d6b97bf4a99..2551f479cab 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -1,7 +1,7 @@ import Combine import Foundation import Yosemite -import enum Networking.DotcomError +import enum Networking.NetworkError import class Networking.UserAgent import class Networking.WordPressOrgNetwork import KeychainAccess @@ -525,7 +525,7 @@ private extension DefaultStoresManager { func synchronizeAddOnsGroups(siteID: Int64) { let action = AddOnGroupAction.synchronizeAddOnGroups(siteID: siteID) { result in if let error = result.failure { - if error as? DotcomError == .noRestRoute { + if let networkError = error as? NetworkError, networkError.apiErrorCode == "rest_no_route" { DDLogError("⚠️ Endpoint for add-on groups is unreachable for siteID: \(siteID). WC Product Add-Ons plugin may be missing.") } else { DDLogError("⛔️ Failed to sync add-on groups for siteID: \(siteID). Error: \(error)") From 19511d82439d8b509910de4dabb9352754266c86 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 17:48:59 +0100 Subject: [PATCH 09/19] Reduce access levels --- Modules/Sources/NetworkingCore/Network/NetworkError.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/Sources/NetworkingCore/Network/NetworkError.swift b/Modules/Sources/NetworkingCore/Network/NetworkError.swift index 22176267a27..2324635b450 100644 --- a/Modules/Sources/NetworkingCore/Network/NetworkError.swift +++ b/Modules/Sources/NetworkingCore/Network/NetworkError.swift @@ -37,7 +37,7 @@ public enum NetworkError: Error, Equatable { } /// Response data accompanied the error if available - public var response: Data? { + var response: Data? { switch self { case .notFound(let response): return response @@ -51,7 +51,7 @@ public enum NetworkError: Error, Equatable { } /// Parsed API error details from response data - public var apiErrorDetails: APIErrorDetails? { + var apiErrorDetails: APIErrorDetails? { guard let data = response else { return nil } return try? JSONDecoder().decode(APIErrorDetails.self, from: data) } From 191fa17ef8734d277d8ff5caaa4d4ad6a86838e1 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 25 Aug 2025 18:19:48 +0100 Subject: [PATCH 10/19] Fix whitespace --- Modules/Sources/Yosemite/Stores/StatsStoreV4.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift b/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift index 2b615c630e8..a9887d08fd1 100644 --- a/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift +++ b/Modules/Sources/Yosemite/Stores/StatsStoreV4.swift @@ -638,7 +638,7 @@ public enum SiteStatsStoreError: Error { // Maps to the old .noStatsPermission case self = .noPermission case "invalid_blog": - // Maps to the old .statsModuleDisabled case + // Maps to the old .statsModuleDisabled case self = .statsModuleDisabled case "unknown_token": // Maps to the old .jetpackNotConnected case From 1a3222d7589efc7141458c3e47ed1c23178cfec6 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 1 Sep 2025 14:03:27 +0100 Subject: [PATCH 11/19] Use NetworkError wrapper in CardReaderConfigTests --- .../CommonReaderConfigProviderTests.swift | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Modules/Tests/YosemiteTests/Tools/CommonReaderConfigProviderTests.swift b/Modules/Tests/YosemiteTests/Tools/CommonReaderConfigProviderTests.swift index 2b744d8e40a..8c26fd4051c 100644 --- a/Modules/Tests/YosemiteTests/Tools/CommonReaderConfigProviderTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/CommonReaderConfigProviderTests.swift @@ -10,8 +10,13 @@ struct CommonReaderConfigProviderTests { let adminURL = "https://example.com/wp-admin/set-address" let mockRemote = MockCardReaderCapableRemote( resultForDefaultReaderLocation: .failure( - DotcomError.unknown(code: "store_address_is_incomplete", - message: adminURL))) + NetworkError.from( + dotcomError: DotcomError.unknown( + code: "store_address_is_incomplete", + message: adminURL), + originalNetworkError: NetworkError.unacceptableStatusCode( + statusCode: 400, + response: nil)))) let sut = CommonReaderConfigProvider(siteID: 123, readerConfigRemote: mockRemote) @@ -29,8 +34,13 @@ struct CommonReaderConfigProviderTests { @Test func fetchDefaultLocationID_returns_invalidPostalCode_error_when_jetpack_site_has_no_postcode() async throws { let mockRemote = MockCardReaderCapableRemote( resultForDefaultReaderLocation: .failure( - DotcomError.unknown(code: "postal_code_invalid", - message: ""))) + NetworkError.from( + dotcomError: DotcomError.unknown( + code: "postal_code_invalid", + message: ""), + originalNetworkError: NetworkError.unacceptableStatusCode( + statusCode: 400, + response: nil)))) let sut = CommonReaderConfigProvider(siteID: 123, readerConfigRemote: mockRemote) From 7b4c620e6f8398d740df2a54d577a74780866f34 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 1 Sep 2025 15:30:27 +0100 Subject: [PATCH 12/19] Update Yosemite tests to use wrapped dotcomerror --- .../Stores/CardPresentPaymentStoreTests.swift | 4 +- .../Stores/JustInTimeMessageStoreTests.swift | 10 ++-- .../Stores/MediaStoreTests.swift | 36 ++++++++----- .../Stores/ProductCategoryStoreTests.swift | 10 +++- .../Stores/ProductStoreTests.swift | 18 +++++-- .../YosemiteTests/Stores/SiteStoreTests.swift | 53 +++++++++++++------ .../SiteVisitStatsStoreErrorTests.swift | 15 ++++-- .../Stores/WooShippingStoreTests.swift | 4 +- .../WooPaymentsPayoutServiceTests.swift | 7 ++- 9 files changed, 111 insertions(+), 46 deletions(-) diff --git a/Modules/Tests/YosemiteTests/Stores/CardPresentPaymentStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/CardPresentPaymentStoreTests.swift index 364ef86d1b0..9f5c5ecc113 100644 --- a/Modules/Tests/YosemiteTests/Stores/CardPresentPaymentStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/CardPresentPaymentStoreTests.swift @@ -497,7 +497,9 @@ final class CardPresentPaymentStoreTests: XCTestCase { XCTAssert(viewStorage.countObjects(ofType: Storage.WCPayCharge.self, matching: nil) == 2) network.simulateError(requestUrlSuffix: "payments/charges/\(sampleErrorChargeID)", - error: DotcomError.unknown(code: "beep", message: "boop")) + error: NetworkError.from( + dotcomError: DotcomError.unknown(code: "beep", message: "boop"), + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil))) let _: Result = waitFor { [self] promise in let action = CardPresentPaymentAction.fetchWCPayCharge(siteID: self.sampleSiteID, chargeID: self.sampleErrorChargeID, onCompletion: { result in diff --git a/Modules/Tests/YosemiteTests/Stores/JustInTimeMessageStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/JustInTimeMessageStoreTests.swift index b0f5c2ce357..cafda8fdd56 100644 --- a/Modules/Tests/YosemiteTests/Stores/JustInTimeMessageStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/JustInTimeMessageStoreTests.swift @@ -84,7 +84,9 @@ final class JustInTimeMessageStoreTests: XCTestCase { func test_loadMessage_then_it_returns_error_upon_response_error() { // Given a stubbed generic-error network response - network.simulateError(requestUrlSuffix: "jetpack/v4/jitm", error: DotcomError.noRestRoute) + network.simulateError(requestUrlSuffix: "jetpack/v4/jitm", error: NetworkError.from( + dotcomError: DotcomError.noRestRoute, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 404, response: nil))) // When dispatching a `loadAllInboxNotes` action let result = waitFor { promise in @@ -100,9 +102,9 @@ final class JustInTimeMessageStoreTests: XCTestCase { // Then no inbox notes should be stored XCTAssert(result.isFailure) guard case .failure(let error) = result, - let error = error as? DotcomError else { - return XCTFail("Expected error not recieved") + let networkError = error as? NetworkError else { + return XCTFail("Expected NetworkError not received") } - assertEqual(DotcomError.noRestRoute, error) + XCTAssertEqual(networkError.apiErrorCode, "rest_no_route") } } diff --git a/Modules/Tests/YosemiteTests/Stores/MediaStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/MediaStoreTests.swift index 3f247db2b43..4fd4a4497e6 100644 --- a/Modules/Tests/YosemiteTests/Stores/MediaStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/MediaStoreTests.swift @@ -72,7 +72,9 @@ final class MediaStoreTests: XCTestCase { // Given let mediaID: Int64 = 22 let remote = MockMediaRemote() - remote.whenLoadingMedia(siteID: sampleSiteID, thenReturn: .failure(DotcomError.unauthorized)) + remote.whenLoadingMedia(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unauthorized, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)))) let mediaStore = MediaStore(dispatcher: dispatcher, storageManager: storageManager, network: network, @@ -92,8 +94,8 @@ final class MediaStoreTests: XCTestCase { // Then XCTAssertEqual(remote.invocations, [.loadMedia(siteID: sampleSiteID, mediaID: mediaID)]) - let error = try XCTUnwrap(result.failure as? DotcomError) - XCTAssertEqual(error, .unauthorized) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "unauthorized") } // MARK: test cases for `MediaAction.retrieveMediaLibrary` @@ -310,7 +312,9 @@ final class MediaStoreTests: XCTestCase { func test_retrieveMediaLibrary_from_jcp_site_returns_error_upon_empty_response() throws { // Given let remote = MockMediaRemote() - remote.whenLoadingMediaLibrary(siteID: sampleSiteID, thenReturn: .failure(DotcomError.unauthorized)) + remote.whenLoadingMediaLibrary(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unauthorized, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)))) let mediaStore = MediaStore(dispatcher: dispatcher, storageManager: storageManager, network: network, @@ -332,8 +336,8 @@ final class MediaStoreTests: XCTestCase { // Then XCTAssertEqual(remote.invocations, [.loadMediaLibrary(siteID: sampleSiteID)]) - let error = try XCTUnwrap(result.failure as? DotcomError) - XCTAssertEqual(error, .unauthorized) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "unauthorized") } // MARK: test cases for `MediaAction.uploadMedia` @@ -423,7 +427,9 @@ final class MediaStoreTests: XCTestCase { }() let remote = MockMediaRemote() - remote.whenUploadingMedia(siteID: sampleSiteID, thenReturn: .failure(DotcomError.unauthorized)) + remote.whenUploadingMedia(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unauthorized, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)))) let mediaStore = createMediaStoreAndExportableMedia(at: targetURL, fileManager: fileManager, remote: remote) @@ -543,7 +549,9 @@ final class MediaStoreTests: XCTestCase { }() let remote = MockMediaRemote() - remote.whenUploadingMedia(siteID: sampleSiteID, thenReturn: .failure(DotcomError.unauthorized)) + remote.whenUploadingMedia(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unauthorized, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)))) let mediaStore = createMediaStoreAndExportableMedia(at: targetURL, fileManager: fileManager, remote: remote) @@ -566,8 +574,8 @@ final class MediaStoreTests: XCTestCase { // Then XCTAssertEqual(remote.invocations, [.uploadMedia(siteID: sampleSiteID)]) - let error = try XCTUnwrap(result.failure as? DotcomError) - XCTAssertEqual(error, .unauthorized) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "unauthorized") } // MARK: test cases for `MediaAction.uploadFile` @@ -666,7 +674,9 @@ final class MediaStoreTests: XCTestCase { func test_updateProductID_returns_error_upon_response_error() throws { // Given let remote = MockMediaRemote() - remote.whenUpdatingProductID(siteID: sampleSiteID, thenReturn: .failure(DotcomError.unauthorized)) + remote.whenUpdatingProductID(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unauthorized, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)))) let mediaStore = MediaStore(dispatcher: dispatcher, storageManager: storageManager, network: network, @@ -683,8 +693,8 @@ final class MediaStoreTests: XCTestCase { } // Then - let error = try XCTUnwrap(result.failure as? DotcomError) - XCTAssertEqual(error, .unauthorized) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "unauthorized") } func test_toMedia_converts_rendered_title_to_file_name_if_media_detail_is_not_available() { diff --git a/Modules/Tests/YosemiteTests/Stores/ProductCategoryStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/ProductCategoryStoreTests.swift index c909c19184b..a4642dfcae7 100644 --- a/Modules/Tests/YosemiteTests/Stores/ProductCategoryStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/ProductCategoryStoreTests.swift @@ -349,7 +349,15 @@ final class ProductCategoryStoreTests: XCTestCase { func test_synchronizeProductCategory_fails_with_resourceDoesNotExist_then_it_provides_right_error() { let categoryID: Int64 = 123 - network.simulateError(requestUrlSuffix: "products/categories/\(categoryID)", error: DotcomError.resourceDoesNotExist) + let errorData = + """ + { + "code": "resource_not_found", + "message": "Resource does not exist" + } + """.data(using: .utf8)! + let networkError = NetworkError.notFound(response: errorData) + network.simulateError(requestUrlSuffix: "products/categories/\(categoryID)", error: networkError) let retrievedError: Error? = waitFor { [weak self] promise in guard let self = self else { diff --git a/Modules/Tests/YosemiteTests/Stores/ProductStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/ProductStoreTests.swift index f3d6c466dd0..8d9c8dd2cda 100644 --- a/Modules/Tests/YosemiteTests/Stores/ProductStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/ProductStoreTests.swift @@ -134,7 +134,9 @@ final class ProductStoreTests: XCTestCase { func test_addProduct_returns_error_upon_network_error() { // Arrange let remote = MockProductsRemote() - remote.whenAddingProduct(siteID: sampleSiteID, thenReturn: .failure(DotcomError.requestFailed)) + remote.whenAddingProduct(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.requestFailed, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)))) let productStore = ProductStore(dispatcher: dispatcher, storageManager: storageManager, network: network, remote: remote) // Action @@ -236,7 +238,9 @@ final class ProductStoreTests: XCTestCase { func test_deleteProduct_returns_error_upon_network_error() { // Arrange let remote = MockProductsRemote() - remote.whenDeletingProduct(siteID: sampleSiteID, thenReturn: .failure(DotcomError.requestFailed)) + remote.whenDeletingProduct(siteID: sampleSiteID, thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.requestFailed, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)))) let productStore = ProductStore(dispatcher: dispatcher, storageManager: storageManager, network: network, remote: remote) // Action @@ -763,10 +767,16 @@ final class ProductStoreTests: XCTestCase { productStore.upsertStoredProduct(readOnlyProduct: sampleProduct(), in: viewStorage) XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Product.self), 1) - let dotComError = DotcomError.unknown(code: ProductLoadError.ErrorCode.invalidID.rawValue, message: nil) + let errorData = + """ + {"code": "\(ProductLoadError.ErrorCode.invalidID.rawValue)", + "message": "Product not found"} + """ + .data(using: .utf8)! + let networkError = NetworkError.unacceptableStatusCode(statusCode: 404, response: errorData) // Action - network.simulateError(requestUrlSuffix: "products/282", error: dotComError) + network.simulateError(requestUrlSuffix: "products/282", error: networkError) let result: Result = waitFor { promise in let action = ProductAction.retrieveProduct(siteID: self.sampleSiteID, productID: self.sampleProductID) { result in promise(result) diff --git a/Modules/Tests/YosemiteTests/Stores/SiteStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/SiteStoreTests.swift index cdf0aecdf6d..12c6bfe00e5 100644 --- a/Modules/Tests/YosemiteTests/Stores/SiteStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/SiteStoreTests.swift @@ -1,4 +1,5 @@ import XCTest +import enum Networking.NetworkError import enum Networking.DotcomError import enum Networking.SiteCreationError import enum Networking.WordPressApiError @@ -125,8 +126,15 @@ final class SiteStoreTests: XCTestCase { func test_createSite_returns_domainExists_error_on_Dotcom_blog_name_exists_error() throws { // Given + let errorData = + """ + { + "code": "blog_name_exists", + "message": "Sorry, that site already exists!" + } + """.data(using: .utf8)! remote.whenCreatingSite(thenReturn: .failure( - DotcomError.unknown(code: "blog_name_exists", message: "Sorry, that site already exists!") + NetworkError.unacceptableStatusCode(statusCode: 400, response: errorData) )) // When @@ -145,9 +153,15 @@ final class SiteStoreTests: XCTestCase { func test_createSite_returns_invalidDomain_error_on_Dotcom_blog_name_error() throws { // Given + let errorData = """ +{ + "code": "blog_name_only_lowercase_letters_and_numbers", + "message": "Site names can only contain lowercase letters (a-z) and numbers." +} +""" + .data(using: .utf8)! remote.whenCreatingSite(thenReturn: .failure( - DotcomError.unknown(code: "blog_name_only_lowercase_letters_and_numbers", - message: "Site names can only contain lowercase letters (a-z) and numbers.") + NetworkError.unacceptableStatusCode(statusCode: 400, response: errorData) )) // When @@ -232,7 +246,9 @@ final class SiteStoreTests: XCTestCase { func test_enableFreeTrial_returns_error_on_failure() throws { // Given - remote.whenEnablingFreeTrial(thenReturn: .failure(DotcomError.unknown(code: "error", message: nil))) + remote.whenEnablingFreeTrial(thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unknown(code: "error", message: nil), + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)))) // When let result = waitFor { promise in @@ -242,8 +258,8 @@ final class SiteStoreTests: XCTestCase { } // Then - let error = try XCTUnwrap(result.failure) - XCTAssertEqual(error as? DotcomError, .unknown(code: "error", message: nil)) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "error") } // MARK: - `updateSiteTitle` @@ -270,7 +286,9 @@ final class SiteStoreTests: XCTestCase { func test_updateSiteTitle_returns_error_on_failure() throws { // Given let siteID: Int64 = 123 - remote.whenUpdatingSiteTitle(thenReturn: .failure(DotcomError.unknown(code: "error", message: nil))) + remote.whenUpdatingSiteTitle(thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unknown(code: "error", message: nil), + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)))) // When let result = waitFor { promise in @@ -281,8 +299,8 @@ final class SiteStoreTests: XCTestCase { // Then XCTAssertFalse(result.isSuccess) - let error = try XCTUnwrap(result.failure) - XCTAssertEqual(error as? DotcomError, .unknown(code: "error", message: nil)) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "error") } // MARK: - `uploadStoreProfilerAnswers` @@ -307,7 +325,9 @@ final class SiteStoreTests: XCTestCase { func test_uploadStoreProfilerAnswers_returns_error_on_failure() throws { // Given - remote.whenUploadingStoreProfilerAnswers(thenReturn: .failure(DotcomError.unknown(code: "error", message: nil))) + remote.whenUploadingStoreProfilerAnswers(thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unknown(code: "error", message: nil), + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)))) // When let result = waitFor { promise in @@ -320,8 +340,8 @@ final class SiteStoreTests: XCTestCase { } // Then - let error = try XCTUnwrap(result.failure) - XCTAssertEqual(error as? DotcomError, .unknown(code: "error", message: nil)) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "error") } // MARK: - `syncSiteByDomain` @@ -348,9 +368,10 @@ final class SiteStoreTests: XCTestCase { func test_syncSiteByDomain_returns_error_on_failure() throws { // Given - let siteID: Int64 = 123 let domain = "example.com" - remote.whenLoadingSite(thenReturn: .failure(DotcomError.unknown(code: "error", message: nil))) + remote.whenLoadingSite(thenReturn: .failure(NetworkError.from( + dotcomError: DotcomError.unknown(code: "error", message: nil), + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)))) // When let result = waitFor { promise in @@ -361,8 +382,8 @@ final class SiteStoreTests: XCTestCase { // Then XCTAssertFalse(result.isSuccess) - let error = try XCTUnwrap(result.failure) - XCTAssertEqual(error as? DotcomError, .unknown(code: "error", message: nil)) + let error = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(error.apiErrorCode, "error") } } diff --git a/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift b/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift index 077bc50a692..84837acff9a 100644 --- a/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift @@ -4,24 +4,31 @@ import XCTest class SiteStatsStoreErrorTests: XCTestCase { func testNoPermissionError() { - let remoteError = DotcomError.noStatsPermission + let remoteError = NetworkError.from( + dotcomError: DotcomError.noStatsPermission, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)) + let error = SiteStatsStoreError(error: remoteError) XCTAssertEqual(error, .noPermission) } func testStatsModuleDisabledError() { - let remoteError = DotcomError.statsModuleDisabled + let remoteError = NetworkError.from( + dotcomError: DotcomError.statsModuleDisabled, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)) let error = SiteStatsStoreError(error: remoteError) XCTAssertEqual(error, .statsModuleDisabled) } func testOtherDotcomError() { - let remoteError = DotcomError.unknown(code: "invalid_blog", message: "This blog does not have Jetpack connected") + let remoteError = NetworkError.from( + dotcomError: DotcomError.unknown(code: "invalid_blog", message: "This blog does not have Jetpack connected"), + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)) let error = SiteStatsStoreError(error: remoteError) XCTAssertEqual(error, .unknown) } - func testNonDotcomRemoteError() { + func testNonNetworkRemoteError() { let remoteError = NSError(domain: "Woo", code: 404, userInfo: nil) let error = SiteStatsStoreError(error: remoteError) XCTAssertEqual(error, .unknown) diff --git a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift index 7505e4ac2b7..7e8546169ef 100644 --- a/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/WooShippingStoreTests.swift @@ -193,7 +193,9 @@ final class WooShippingStoreTests: XCTestCase { func test_deletePackage_returns_error_on_failure() throws { // Given let remote = MockWooShippingRemote() - let error = DotcomError.requestFailed + let error = NetworkError.from( + dotcomError: DotcomError.requestFailed, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)) remote.whenDeletePackage(siteID: sampleSiteID, thenReturn: .failure(error)) let store = WooShippingStore(dispatcher: dispatcher, storageManager: storageManager, network: network, remote: remote) diff --git a/Modules/Tests/YosemiteTests/Tools/Payments/WooPaymentsPayoutServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/Payments/WooPaymentsPayoutServiceTests.swift index ecbb0bba636..833c8d6d20d 100644 --- a/Modules/Tests/YosemiteTests/Tools/Payments/WooPaymentsPayoutServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/Payments/WooPaymentsPayoutServiceTests.swift @@ -99,7 +99,9 @@ final class WooPaymentsPayoutServiceTests: XCTestCase { func testFetchPayoutsOverviewError() async { // Given - let mockError = DotcomError.noRestRoute + let mockError = NetworkError.from( + dotcomError: DotcomError.noRestRoute, + originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 404, response: nil)) mockNetwork.simulateError(requestUrlSuffix: "payments/deposits/overview-all", error: mockError) do { @@ -108,7 +110,8 @@ final class WooPaymentsPayoutServiceTests: XCTestCase { XCTFail("Expected an error, but the call succeeded.") } catch { // Then - XCTAssertEqual(error as? DotcomError, mockError) + let networkError = error as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "rest_no_route") } } } From 1c169e256fe6b3c761f52368df01574f25f12ae9 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 11:44:17 +0100 Subject: [PATCH 13/19] Fix code for `other` error --- .../YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift b/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift index 84837acff9a..71ad17db824 100644 --- a/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/SiteVisitStatsStoreErrorTests.swift @@ -22,7 +22,7 @@ class SiteStatsStoreErrorTests: XCTestCase { func testOtherDotcomError() { let remoteError = NetworkError.from( - dotcomError: DotcomError.unknown(code: "invalid_blog", message: "This blog does not have Jetpack connected"), + dotcomError: DotcomError.unknown(code: "something_else", message: "This blog does not have a coffee machine connected"), originalNetworkError: NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)) let error = SiteStatsStoreError(error: remoteError) XCTAssertEqual(error, .unknown) From 00232b3e3e75215b3fe79aac0928d3507f34d9a6 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 13:42:46 +0100 Subject: [PATCH 14/19] =?UTF-8?q?Ensure=20=E2=80=9Csuccess=E2=80=9D=20path?= =?UTF-8?q?=20errors=20are=20NetworkErrors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NetworkingCore/Model/DotcomError.swift | 14 ++++++++-- .../NetworkingCore/Network/NetworkError.swift | 28 +++++++++++++++++-- .../NetworkingCore/Remote/Remote.swift | 26 +++++++++++++++++ .../NetworkingTests/Remote/RemoteTests.swift | 22 +++++++++++---- 4 files changed, 80 insertions(+), 10 deletions(-) diff --git a/Modules/Sources/NetworkingCore/Model/DotcomError.swift b/Modules/Sources/NetworkingCore/Model/DotcomError.swift index d15fa6fe899..7b11a3df81d 100644 --- a/Modules/Sources/NetworkingCore/Model/DotcomError.swift +++ b/Modules/Sources/NetworkingCore/Model/DotcomError.swift @@ -173,7 +173,14 @@ public func ==(lhs: DotcomError, rhs: DotcomError) -> Bool { public extension NetworkError { /// Creates a NetworkError from a DotcomError, preserving the original NetworkError's status code and response data /// This is used in Remote.mapNetworkError to maintain real HTTP status codes while adding parsed API error details - static func from(dotcomError: DotcomError, originalNetworkError: NetworkError) -> NetworkError { + static func from(dotcomError: DotcomError, originalNetworkError: NetworkError? = nil) -> NetworkError { + guard let originalNetworkError = originalNetworkError else { + // No original NetworkError - this is likely from successful HTTP response with API error content + let (code, message) = dotcomError.getCodeAndMessage() + let errorData = dotcomError.createEnhancedErrorResponseData(originalResponse: nil) + return .apiError(code: code, message: message, response: errorData) + } + let enhancedErrorData = dotcomError.createEnhancedErrorResponseData(originalResponse: originalNetworkError.response) switch originalNetworkError { @@ -183,6 +190,9 @@ public extension NetworkError { return .timeout(response: enhancedErrorData) case let .unacceptableStatusCode(statusCode, _): return .unacceptableStatusCode(statusCode: statusCode, response: enhancedErrorData) + case let .apiError(_, _, response): + let (code, message) = dotcomError.getCodeAndMessage() + return .apiError(code: code, message: message, response: response) case .invalidURL: return .invalidURL case .invalidCookieNonce: @@ -208,7 +218,7 @@ private extension DotcomError { } /// Extracts the appropriate error code and message for each DotcomError case - private func getCodeAndMessage() -> (code: String, message: String) { + func getCodeAndMessage() -> (code: String, message: String) { switch self { case .empty: return ("empty", "Empty response") diff --git a/Modules/Sources/NetworkingCore/Network/NetworkError.swift b/Modules/Sources/NetworkingCore/Network/NetworkError.swift index 2324635b450..cd9e531c1a7 100644 --- a/Modules/Sources/NetworkingCore/Network/NetworkError.swift +++ b/Modules/Sources/NetworkingCore/Network/NetworkError.swift @@ -22,6 +22,9 @@ public enum NetworkError: Error, Equatable { /// Error for REST API requests with invalid cookie nonce case invalidCookieNonce + /// API-level error parsed from a successful HTTP response + case apiError(code: String, message: String?, response: Data? = nil) + /// The HTTP response code of the network error, for cases that are deducted from the status code. public var responseCode: Int? { switch self { @@ -31,7 +34,7 @@ public enum NetworkError: Error, Equatable { return StatusCode.timeout case let .unacceptableStatusCode(statusCode, _): return statusCode - default: + case .apiError, .invalidURL, .invalidCookieNonce: return nil } } @@ -45,6 +48,8 @@ public enum NetworkError: Error, Equatable { return response case .unacceptableStatusCode(_, let response): return response + case .apiError(_, _, let response): + return response case .invalidURL, .invalidCookieNonce: return nil } @@ -58,12 +63,22 @@ public enum NetworkError: Error, Equatable { /// API error code from response, if available public var apiErrorCode: String? { - return apiErrorDetails?.code + switch self { + case .apiError(let code, _, _): + return code + default: + return apiErrorDetails?.code + } } /// API error message from response, if available public var apiErrorMessage: String? { - return apiErrorDetails?.message + switch self { + case .apiError(_, let message, _): + return message + default: + return apiErrorDetails?.message + } } } @@ -134,6 +149,13 @@ extension NetworkError: CustomStringConvertible { "NetworkError.invalidCookieNonce", value: "Sorry, your session has expired. Please log in again.", comment: "Error message when session cookie has expired.") + case let .apiError(code, message, _): + let messageText = message ?? "" + let format = NSLocalizedString( + "NetworkError.apiError", + value: "API Error: [%1$@] %2$@", + comment: "Error message for API-level errors. %1$@ is the error code, %2$@ is the error message") + return String.localizedStringWithFormat(format, code, messageText) } } } diff --git a/Modules/Sources/NetworkingCore/Remote/Remote.swift b/Modules/Sources/NetworkingCore/Remote/Remote.swift index 366833b4619..c18c2f32d10 100644 --- a/Modules/Sources/NetworkingCore/Remote/Remote.swift +++ b/Modules/Sources/NetworkingCore/Remote/Remote.swift @@ -35,6 +35,9 @@ open class Remote: NSObject { let validator = request.responseDataValidator() try validator.validate(data: data) continuation.resume() + } catch let dotcomError as DotcomError { + self.handleResponseError(error: dotcomError, for: request) + continuation.resume(throwing: NetworkError.from(dotcomError: dotcomError)) } catch { self.handleResponseError(error: error, for: request) continuation.resume(throwing: error) @@ -62,6 +65,9 @@ open class Remote: NSObject { try validator.validate(data: data) let document = try JSONDecoder().decode(T.self, from: data) continuation.resume(returning: document) + } catch let dotcomError as DotcomError { + self.handleResponseError(error: dotcomError, for: request) + continuation.resume(throwing: NetworkError.from(dotcomError: dotcomError)) } catch { self.handleResponseError(error: error, for: request) self.handleDecodingError(error: error, for: request, entityName: "\(T.self)") @@ -102,6 +108,10 @@ open class Remote: NSObject { try validator.validate(data: data) let parsed = try mapper.map(response: data) completion(parsed, nil) + } catch let dotcomError as DotcomError { + self.handleResponseError(error: dotcomError, for: request) + let networkError = NetworkError.from(dotcomError: dotcomError) + completion(nil, networkError) } catch { self.handleResponseError(error: error, for: request) self.handleDecodingError(error: error, for: request, entityName: "\(M.Output.self)") @@ -135,6 +145,10 @@ open class Remote: NSObject { try validator.validate(data: data) let parsed = try mapper.map(response: data) completion(.success(parsed)) + } catch let dotcomError as DotcomError { + self.handleResponseError(error: dotcomError, for: request) + let networkError = NetworkError.from(dotcomError: dotcomError) + completion(.failure(networkError)) } catch { self.handleResponseError(error: error, for: request) self.handleDecodingError(error: error, for: request, entityName: "\(M.Output.self)") @@ -183,6 +197,14 @@ open class Remote: NSObject { self?.handleDecodingError(error: decodingError, for: request, entityName: "\(M.Output.self)") } }) + .map { (result: Result) -> Result in + guard case let .failure(error) = result, + let dotcomError = error as? DotcomError + else { + return result + } + return .failure(NetworkError.from(dotcomError: dotcomError)) + } .eraseToAnyPublisher() } @@ -256,6 +278,10 @@ private extension Remote { do { try validator.validate(data: data) return try mapper.map(response: data) + } catch let dotcomError as DotcomError { + DDLogError("<> Mapping Error: \(dotcomError)") + handleResponseError(error: dotcomError, for: request) + throw NetworkError.from(dotcomError: dotcomError) } catch { DDLogError("<> Mapping Error: \(error)") handleDecodingError(error: error, for: request, entityName: "\(M.Output.self)") diff --git a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift index af0a08379aa..b98300cdecd 100644 --- a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift @@ -258,8 +258,8 @@ final class RemoteTests: XCTestCase { // Then XCTAssertTrue(result.isFailure) - let networkError = result.failure as? NetworkError - XCTAssertEqual(networkError?.apiErrorCode, "http_request_failed") + let networkError = try XCTUnwrap(result.failure as? NetworkError) + XCTAssertEqual(networkError.apiErrorCode, "http_request_failed") } /// Verifies that dotcom v1.1 request parses DotcomError @@ -273,7 +273,11 @@ final class RemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "mock", filename: "timeout_error") - await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { $0 is DotcomError }) + await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { + guard let networkError = $0 as? NetworkError, + case .apiError(let code, _, _) = networkError else { return false } + return code == "http_request_failed" + }) } /// Verifies that dotcom v1.1 request doesn't parse WordPressApiError @@ -303,7 +307,11 @@ final class RemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "mock", filename: "timeout_error") - await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { $0 is DotcomError }) + await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { + guard let networkError = $0 as? NetworkError, + case .apiError(let code, _, _) = networkError else { return false } + return code == "http_request_failed" + }) } /// Verifies that dotcom v1.2 request doesn't parse WordPressApiError @@ -399,7 +407,11 @@ final class RemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "mock", filename: "timeout_error") - await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { $0 is DotcomError }) + await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { + guard let networkError = $0 as? NetworkError, + case .apiError(let code, _, _) = networkError else { return false } + return code == "http_request_failed" + }) } /// Verifies that Jetpack request doesn't parse WordPressApiError From a106c00d9f2a7eb7bbaa9a8479272b3527fcf461 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 14:15:25 +0100 Subject: [PATCH 15/19] Remove unnecessary tests --- .../NetworkingTests/Remote/RemoteTests.swift | 142 ------------------ 1 file changed, 142 deletions(-) diff --git a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift index b98300cdecd..5de445000de 100644 --- a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift @@ -676,37 +676,6 @@ final class RemoteTests: XCTestCase { } // MARK: Mapping `NetworkError` - - /// Verifies that `enqueue:mapper:` (with `Result`) maps an error from `responseData` when error has proper response data - /// - func test_enqueue_request_with_result_throws_DotcomError_from_NetworkError_with_response_data() throws { - // Given - let network = MockNetwork() - let mapper = DummyMapper() - let remote = Remote(network: network) - - let data = Loader.contentsOf("timeout_error") - let errorsWithResponse: [NetworkError] = [ - .notFound(response: data), - .timeout(response: data), - .unacceptableStatusCode(statusCode: 403, response: data) - ] - for error in errorsWithResponse { - network.simulateError(requestUrlSuffix: "something", error: error) - - // When - let result: Result = waitFor { promise in - remote.enqueue(self.request, mapper: mapper) { result in - promise(result) - } - } - - // Then - XCTAssertTrue(result.isFailure) - XCTAssertTrue(try XCTUnwrap(result.failure) is DotcomError) - } - } - /// Verifies that `enqueue:mapper:` (with `Result`) throws same error when NetworkError does not have proper response data /// func test_enqueue_request_with_result_throws_same_errors_for_NetworkError_without_response_data() throws { @@ -739,36 +708,6 @@ final class RemoteTests: XCTestCase { } } - /// Verifies that `enqueuePublisher` maps an error from `responseData` when error has proper response data - /// - func test_enqueuePublisher_throws_DotcomError_from_NetworkError_with_response() throws { - // Given - let network = MockNetwork() - let mapper = DummyMapper() - let remote = Remote(network: network) - - let data = Loader.contentsOf("timeout_error") - let errorsWithResponse: [NetworkError] = [ - .notFound(response: data), - .timeout(response: data), - .unacceptableStatusCode(statusCode: 403, response: data) - ] - for error in errorsWithResponse { - network.simulateError(requestUrlSuffix: "something", error: error) - - // When - let result: Result = waitFor { promise in - remote.enqueue(self.request, mapper: mapper).sink { result in - promise(result) - }.store(in: &self.cancellables) - } - - // Then - XCTAssertTrue(result.isFailure) - XCTAssertTrue(try XCTUnwrap(result.failure) is DotcomError) - } - } - /// Verifies that `enqueuePublisher` throws same error when NetworkError does not have response data. /// func test_enqueuePublisher_throws_same_error_for_NetworkError_without_response_data() throws { @@ -801,33 +740,6 @@ final class RemoteTests: XCTestCase { } } - /// Verifies that `enqueue` async version maps an error from `responseData` when error has proper response data. - /// - func test_enqueue_async_throws_DotcomError_from_NetworkError_with_proper_response_data() async throws { - // Given - let network = MockNetwork() - let remote = Remote(network: network) - - let data = Loader.contentsOf("timeout_error") - let errorsWithResponse: [NetworkError] = [ - .notFound(response: data), - .timeout(response: data), - .unacceptableStatusCode(statusCode: 403, response: data) - ] - - for error in errorsWithResponse { - network.simulateError(requestUrlSuffix: "something", error: error) - - // When - do { - _ = try await remote.enqueue(request) - } catch { - // Then - XCTAssertTrue(error is DotcomError) - } - } - } - /// Verifies that `enqueue` async version throws same error when NetworkError doesn't have proper response data /// func test_enqueue_async_throws_same_error_for_NetworkError_without_response_data() async throws { @@ -855,33 +767,6 @@ final class RemoteTests: XCTestCase { } } - /// Verifies that `enqueue` async version with return type maps an error from `responseData` when error has proper response data - /// - func test_enqueue_async_with_return_type_throws_DotcomError_from_NetworkError_with_proper_response_data() async throws { - // Given - let network = MockNetwork() - let remote = Remote(network: network) - - let data = Loader.contentsOf("timeout_error") - let errorsWithResponse: [NetworkError] = [ - .notFound(response: data), - .timeout(response: data), - .unacceptableStatusCode(statusCode: 403, response: data) - ] - - for error in errorsWithResponse { - network.simulateError(requestUrlSuffix: "something", error: error) - - // When - do { - let _: String = try await remote.enqueue(request) - } catch { - // Then - XCTAssertTrue(error is DotcomError) - } - } - } - /// Verifies that `enqueue` async version with return type throws same error when NetworkError does not have proper response data /// /// @@ -910,33 +795,6 @@ final class RemoteTests: XCTestCase { } } - /// Verifies that `enqueue` async version maps an error from `responseData` when error has proper response data - /// - func test_enqueueWithMapper_async_throws_DotcomError_from_NetworkError_with_proper_response_data() async throws { - // Given - let network = MockNetwork() - let mapper = DummyMapper() - let remote = Remote(network: network) - - let data = Loader.contentsOf("timeout_error") - let errorsWithResponse: [NetworkError] = [ - .notFound(response: data), - .timeout(response: data), - .unacceptableStatusCode(statusCode: 403, response: data) - ] - - for error in errorsWithResponse { - network.simulateError(requestUrlSuffix: "something", error: error) - - // When - do { - _ = try await remote.enqueue(request, mapper: mapper) - } catch { - XCTAssertTrue(error is DotcomError) - } - } - } - /// Verifies that `enqueue` async version throws same error when NetworkError does not have proper response data /// From 78cb337937bba4fca56f4d1a46fd8c1aba053802 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 14:27:34 +0100 Subject: [PATCH 16/19] Update account creation to use wrapped errors --- .../Networking/Remote/AccountRemote.swift | 35 +++++++++---------- .../Networking/Remote/MockAccountRemote.swift | 2 +- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/Modules/Sources/Networking/Remote/AccountRemote.swift b/Modules/Sources/Networking/Remote/AccountRemote.swift index bda44cba057..6fd71701356 100644 --- a/Modules/Sources/Networking/Remote/AccountRemote.swift +++ b/Modules/Sources/Networking/Remote/AccountRemote.swift @@ -189,10 +189,11 @@ public class AccountRemote: Remote, AccountRemoteProtocol { let result: CreateAccountResult = try await enqueue(request) return .success(result) } catch { - guard let dotcomError = error as? DotcomError else { + if let networkError = error as? NetworkError { + return .failure(CreateAccountError(error: networkError)) + } else { return .failure(.unknown(error: error as NSError)) } - return .failure(CreateAccountError(dotcomError: dotcomError)) } } @@ -247,26 +248,22 @@ public enum CreateAccountError: Error, Equatable { case invalidUsername case invalidEmail case invalidPassword(message: String?) - case unexpected(error: DotcomError) + case unexpected(error: NetworkError) case unknown(error: NSError) - /// Decodable Initializer. + /// NetworkError Initializer. /// - init(dotcomError error: DotcomError) { - if case let .unknown(code, message) = error { - switch code { - case Constants.emailExists: - self = .emailExists - case Constants.invalidEmail: - self = .invalidEmail - case Constants.invalidPassword: - self = .invalidPassword(message: message) - case Constants.invalidUsername, Constants.usernameExists: - self = .invalidUsername - default: - self = .unexpected(error: error) - } - } else { + init(error: NetworkError) { + switch error.apiErrorCode { + case Constants.emailExists: + self = .emailExists + case Constants.invalidEmail: + self = .invalidEmail + case Constants.invalidPassword: + self = .invalidPassword(message: error.apiErrorMessage) + case Constants.invalidUsername, Constants.usernameExists: + self = .invalidUsername + default: self = .unexpected(error: error) } } diff --git a/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockAccountRemote.swift b/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockAccountRemote.swift index 8bf44430cff..137df39ad92 100644 --- a/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockAccountRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/Networking/Remote/MockAccountRemote.swift @@ -154,7 +154,7 @@ extension MockAccountRemote: AccountRemoteProtocol { clientSecret: String) async -> Result { guard let result = createAccountResult else { XCTFail("Could not find result for creating an account.") - return .failure(.unexpected(error: .empty)) + return .failure(.unexpected(error: .notFound(response: nil))) } return result } From cb09853e274f871dfaa400fa8c4be90444f7e8be Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 15:25:57 +0100 Subject: [PATCH 17/19] Update tests to use NetworkError, not DotcomError --- .../Remote/WordPressThemeRemote.swift | 4 +- .../Remote/CommentRemoteTests.swift | 7 +-- .../Remote/NotificationsRemoteTests.swift | 16 +++---- .../Remote/ProductsReportsRemoteTests.swift | 2 +- .../Remote/ReportRemoteTests.swift | 2 +- .../Remote/ShipmentsRemoteTests.swift | 44 ++++++++++--------- .../Remote/ShippingLabelRemoteTests.swift | 15 +++---- .../Remote/SiteRemoteTests.swift | 22 ++++++---- .../Remote/WooShippingRemoteTests.swift | 11 +++-- 9 files changed, 63 insertions(+), 60 deletions(-) diff --git a/Modules/Sources/Networking/Remote/WordPressThemeRemote.swift b/Modules/Sources/Networking/Remote/WordPressThemeRemote.swift index bde1e814fff..adfb42e37a7 100644 --- a/Modules/Sources/Networking/Remote/WordPressThemeRemote.swift +++ b/Modules/Sources/Networking/Remote/WordPressThemeRemote.swift @@ -70,8 +70,8 @@ public enum InstallThemeError: Error { case themeAlreadyInstalled init?(_ error: Error) { - guard let dotcomError = error as? DotcomError, - case let .unknown(code, _) = dotcomError else { + guard let networkError = error as? NetworkError, + let code = networkError.apiErrorCode else { return nil } diff --git a/Modules/Tests/NetworkingTests/Remote/CommentRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/CommentRemoteTests.swift index c25430f5ccf..988d3e0a0a1 100644 --- a/Modules/Tests/NetworkingTests/Remote/CommentRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/CommentRemoteTests.swift @@ -142,12 +142,13 @@ class CommentRemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "sites/\(sampleSiteID)/comments/\(sampleCommentID)", filename: "generic_error") remote.moderateComment(siteID: sampleSiteID, commentID: sampleCommentID, status: .untrash) { (updatedStatus, error) in - guard let error = error as? DotcomError else { - XCTFail() + XCTAssertNotNil(error, "Expected an error but got nil") + guard let networkError = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } + XCTAssertEqual(networkError.apiErrorCode, "unauthorized", "Expected apiErrorCode 'unauthorized' but got '\(networkError.apiErrorCode ?? "nil")'") - XCTAssert(error == .unauthorized) XCTAssertNil(updatedStatus) expectation.fulfill() diff --git a/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift index f727fdbe4ed..65e9291256d 100644 --- a/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift @@ -90,12 +90,12 @@ final class NotificationsRemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "notifications/seen", filename: "generic_error") remote.updateLastSeen("") { error in - guard let error = error as? DotcomError else { - XCTFail() + guard let error = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } - - XCTAssert(error == .unauthorized) + XCTAssertEqual(error.apiErrorCode, "unauthorized", "Expected apiErrorCode 'unauthorized' but got '\(error.apiErrorCode ?? "nil")'") + expectation.fulfill() } @@ -127,13 +127,13 @@ final class NotificationsRemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "notifications/read", filename: "generic_error") remote.updateReadStatus(noteIDs: [], read: true) { error in - guard let error = error as? DotcomError else { - XCTFail() + guard let error = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } - XCTAssert(error == .unauthorized) - + XCTAssertEqual(error.apiErrorCode, "unauthorized", "Expected apiErrorCode 'unauthorized' but got '\(error.apiErrorCode ?? "nil")'") + expectation.fulfill() } diff --git a/Modules/Tests/NetworkingTests/Remote/ProductsReportsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/ProductsReportsRemoteTests.swift index fbdd4abf1db..4b906991875 100644 --- a/Modules/Tests/NetworkingTests/Remote/ProductsReportsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/ProductsReportsRemoteTests.swift @@ -56,6 +56,6 @@ class ProductsReportsRemoteTests: XCTestCase { earliestDateToInclude: Date(), latestDateToInclude: Date(), quantity: 5) - }, errorAssert: { ($0 as? DotcomError) == .unauthorized }) + }, errorAssert: { ($0 as? NetworkError)?.apiErrorCode == "unauthorized" }) } } diff --git a/Modules/Tests/NetworkingTests/Remote/ReportRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/ReportRemoteTests.swift index c7e1020f756..4a26b1754f5 100644 --- a/Modules/Tests/NetworkingTests/Remote/ReportRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/ReportRemoteTests.swift @@ -60,6 +60,6 @@ class ReportRemoteTests: XCTestCase { //Then XCTAssertTrue(result.isFailure) - XCTAssertEqual(result.failure as? DotcomError, .unauthorized) + XCTAssertEqual((result.failure as? NetworkError)?.apiErrorCode, "unauthorized") } } diff --git a/Modules/Tests/NetworkingTests/Remote/ShipmentsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/ShipmentsRemoteTests.swift index 60af54669fb..4e7940c04ff 100644 --- a/Modules/Tests/NetworkingTests/Remote/ShipmentsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/ShipmentsRemoteTests.swift @@ -88,13 +88,13 @@ final class ShipmentsRemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "orders/\(sampleOrderID)/shipment-trackings/", filename: "shipment_tracking_plugin_not_active") remote.loadShipmentTrackings(for: sampleSiteID, orderID: sampleOrderID, completion: { (shipmentTrackings, error) in XCTAssertNil(shipmentTrackings) - XCTAssertNotNil(error) + XCTAssertNotNil(error, "Expected an error but got nil") - guard let dotComError = error as? DotcomError else { - XCTFail() + guard let networkError = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } - XCTAssertTrue(dotComError == .noRestRoute) + XCTAssertEqual(networkError.apiErrorCode, "rest_no_route", "Expected apiErrorCode 'rest_no_route' but got '\(networkError.apiErrorCode ?? "nil")'") expectation.fulfill() }) @@ -182,13 +182,14 @@ final class ShipmentsRemoteTests: XCTestCase { dateShipped: "2019-04-01", trackingNumber: "1111") { (shipmentTracking, error) in XCTAssertNil(shipmentTracking) - XCTAssertNotNil(error) + XCTAssertNotNil(error, "Expected an error but got nil") - guard let dotComError = error as? DotcomError else { - XCTFail() + guard let networkError = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } - XCTAssertTrue(dotComError == .noRestRoute) + XCTAssertEqual(networkError.apiErrorCode, "rest_no_route", "Expected apiErrorCode 'rest_no_route' but got '\(networkError.apiErrorCode ?? "nil")'") + expectation.fulfill() } @@ -280,13 +281,12 @@ final class ShipmentsRemoteTests: XCTestCase { trackingURL: "https://somewhere.online.net.com?q=%1$s", dateShipped: "1234") { (shipmentTracking, error) in XCTAssertNil(shipmentTracking) - XCTAssertNotNil(error) + XCTAssertNotNil(error, "Expected an error but got nil") - guard let dotComError = error as? DotcomError else { - XCTFail() - return + guard let networkError = error as? NetworkError else { + return XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") } - XCTAssertTrue(dotComError == .noRestRoute) + XCTAssertEqual(networkError.apiErrorCode, "rest_no_route", "Expected apiErrorCode 'rest_no_route' but got '\(networkError.apiErrorCode ?? "nil")'") expectation.fulfill() } @@ -360,13 +360,14 @@ final class ShipmentsRemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "orders/\(sampleOrderID)/shipment-trackings/\(trackingID)", filename: "shipment_tracking_plugin_not_active") remote.deleteShipmentTracking(for: sampleSiteID, orderID: sampleOrderID, trackingID: trackingID) { (shipmentTracking, error) in XCTAssertNil(shipmentTracking) - XCTAssertNotNil(error) + XCTAssertNotNil(error, "Expected an error but got nil") - guard let dotComError = error as? DotcomError else { - XCTFail() + guard let networkError = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } - XCTAssertTrue(dotComError == .noRestRoute) + XCTAssertEqual(networkError.apiErrorCode, "rest_no_route", "Expected apiErrorCode 'rest_no_route' but got '\(networkError.apiErrorCode ?? "nil")'") + expectation.fulfill() } @@ -435,13 +436,14 @@ final class ShipmentsRemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "orders/\(sampleOrderID)/shipment-trackings/providers", filename: "shipment_tracking_plugin_not_active") remote.loadShipmentTrackingProviderGroups(for: sampleSiteID, orderID: sampleOrderID) { (shipmentTrackingGroups, error) in XCTAssertNil(shipmentTrackingGroups) - XCTAssertNotNil(error) + XCTAssertNotNil(error, "Expected an error but got nil") - guard let dotComError = error as? DotcomError else { - XCTFail() + guard let networkError = error as? NetworkError else { + XCTFail("Expected NetworkError but got \(type(of: error)): \(String(describing: error))") return } - XCTAssertTrue(dotComError == .noRestRoute) + XCTAssertEqual(networkError.apiErrorCode, "rest_no_route", "Expected apiErrorCode 'rest_no_route' but got '\(networkError.apiErrorCode ?? "nil")'") + expectation.fulfill() } diff --git a/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift index 75c9e82f8f9..4e4c310f2fe 100644 --- a/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift @@ -99,11 +99,9 @@ final class ShippingLabelRemoteTests: XCTestCase { } // Then - let expectedError = DotcomError - .unknown(code: "wcc_server_error_response", - message: "Error: The WooCommerce Shipping & Tax server returned: Bad Request Unable to request refund. " + - "The parcel has been shipped. ( 400 )") - XCTAssertEqual(result.failure as? DotcomError, expectedError) + let networkError = result.failure as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "wcc_server_error_response") + XCTAssertEqual(networkError?.apiErrorMessage, "Error: The WooCommerce Shipping & Tax server returned: Bad Request Unable to request refund. The parcel has been shipped. ( 400 )") } func test_shippingAddressValidation_returns_address_on_success() throws { @@ -276,10 +274,9 @@ final class ShippingLabelRemoteTests: XCTestCase { } // Then - let expectedError = DotcomError - .unknown(code: "duplicate_custom_package_names_of_existing_packages", - message: "At least one of the new custom packages has the same name as existing packages.") - XCTAssertEqual(result.failure as? DotcomError, expectedError) + let networkError = result.failure as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "duplicate_custom_package_names_of_existing_packages") + XCTAssertEqual(networkError?.apiErrorMessage, "At least one of the new custom packages has the same name as existing packages.") } func test_createPackage_returns_missingPackage_error_with_no_packages() throws { diff --git a/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift index 85587f799cd..727528eeedc 100644 --- a/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift @@ -50,8 +50,10 @@ final class SiteRemoteTests: XCTestCase { await assertThrowsError({ // When _ = try await remote.createSite(name: "Wapuu swags", flow: .onboarding(domain: "wapuu.store")) - }, errorAssert: { ($0 as? DotcomError) == .unknown(code: "blog_name_only_lowercase_letters_and_numbers", - message: "Site names can only contain lowercase letters (a-z) and numbers.") + }, errorAssert: { + guard let networkError = $0 as? NetworkError else { return false } + return networkError.apiErrorCode == "blog_name_only_lowercase_letters_and_numbers" && + networkError.apiErrorMessage == "Site names can only contain lowercase letters (a-z) and numbers." }) } @@ -128,8 +130,9 @@ final class SiteRemoteTests: XCTestCase { // When try await remote.enableFreeTrial(siteID: 134) }, errorAssert: { error in - (error as? DotcomError) == .unknown(code: "no-upgrades-permitted", - message: "You cannot add WordPress.com eCommerce Trial when you already have paid upgrades") + guard let networkError = error as? NetworkError else { return false } + return networkError.apiErrorCode == "no-upgrades-permitted" && + networkError.apiErrorMessage == "You cannot add WordPress.com eCommerce Trial when you already have paid upgrades" }) } @@ -290,7 +293,7 @@ final class SiteRemoteTests: XCTestCase { category: "clothing_and_accessories", countryCode: "US")) }, errorAssert: { error in - (error as? DotcomError) == .unauthorized + (error as? NetworkError)?.apiErrorCode == "unauthorized" }) } @@ -318,10 +321,11 @@ final class SiteRemoteTests: XCTestCase { await assertThrowsError({ // When _ = try await remote.loadSite(domain: domain) - }, errorAssert: { ($0 as? DotcomError) == .unknown( - code: "parse_error", - message: "The Jetpack site is inaccessible or returned an error: parse error (local). not well formed [-32710]" - )}) + }, errorAssert: { + guard let networkError = $0 as? NetworkError else { return false } + return networkError.apiErrorCode == "parse_error" && + networkError.apiErrorMessage == "The Jetpack site is inaccessible or returned an error: parse error (local). not well formed [-32710]" + }) } // MARK: - `finalizeJetpackConnection` diff --git a/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift index 0f2d1896301..6038d35a5b9 100644 --- a/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/WooShippingRemoteTests.swift @@ -102,10 +102,9 @@ final class WooShippingRemoteTests: XCTestCase { } // Then - let expectedError = DotcomError - .unknown(code: "duplicate_custom_package_names_of_existing_packages", - message: "At least one of the new custom packages has the same name as existing packages.") - XCTAssertEqual(result.failure as? DotcomError, expectedError) + let networkError = result.failure as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "duplicate_custom_package_names_of_existing_packages") + XCTAssertEqual(networkError?.apiErrorMessage, "At least one of the new custom packages has the same name as existing packages.") } func test_createPackage_returns_missingPackage_error_with_no_packages() throws { @@ -163,8 +162,8 @@ final class WooShippingRemoteTests: XCTestCase { } // Then - let expectedError = DotcomError.unauthorized - XCTAssertEqual(result.failure as? DotcomError, expectedError) + let networkError = result.failure as? NetworkError + XCTAssertEqual(networkError?.apiErrorCode, "unauthorized") } func test_loadLabelRates_sends_correct_params_and_parses_success_response() throws { From c936d02e9fe86f45fa15fd4ad1119ab37356eda3 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 15:31:13 +0100 Subject: [PATCH 18/19] Fix lint --- .../Remote/NotificationsRemoteTests.swift | 4 ++-- Modules/Tests/NetworkingTests/Remote/RemoteTests.swift | 4 ++-- .../Remote/ShippingLabelRemoteTests.swift | 6 +++++- .../Tests/NetworkingTests/Remote/SiteRemoteTests.swift | 10 +++++----- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift index 65e9291256d..72d8fe53460 100644 --- a/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/NotificationsRemoteTests.swift @@ -95,7 +95,7 @@ final class NotificationsRemoteTests: XCTestCase { return } XCTAssertEqual(error.apiErrorCode, "unauthorized", "Expected apiErrorCode 'unauthorized' but got '\(error.apiErrorCode ?? "nil")'") - + expectation.fulfill() } @@ -133,7 +133,7 @@ final class NotificationsRemoteTests: XCTestCase { } XCTAssertEqual(error.apiErrorCode, "unauthorized", "Expected apiErrorCode 'unauthorized' but got '\(error.apiErrorCode ?? "nil")'") - + expectation.fulfill() } diff --git a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift index 5de445000de..8c2cd73619a 100644 --- a/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/RemoteTests.swift @@ -307,7 +307,7 @@ final class RemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "mock", filename: "timeout_error") - await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { + await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { guard let networkError = $0 as? NetworkError, case .apiError(let code, _, _) = networkError else { return false } return code == "http_request_failed" @@ -407,7 +407,7 @@ final class RemoteTests: XCTestCase { network.simulateResponse(requestUrlSuffix: "mock", filename: "timeout_error") - await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { + await assertThrowsError({ _ = try await remote.enqueue(request, mapper: mapper)}, errorAssert: { guard let networkError = $0 as? NetworkError, case .apiError(let code, _, _) = networkError else { return false } return code == "http_request_failed" diff --git a/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift index 4e4c310f2fe..a34b36d5087 100644 --- a/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/ShippingLabelRemoteTests.swift @@ -101,7 +101,11 @@ final class ShippingLabelRemoteTests: XCTestCase { // Then let networkError = result.failure as? NetworkError XCTAssertEqual(networkError?.apiErrorCode, "wcc_server_error_response") - XCTAssertEqual(networkError?.apiErrorMessage, "Error: The WooCommerce Shipping & Tax server returned: Bad Request Unable to request refund. The parcel has been shipped. ( 400 )") + XCTAssertEqual( + networkError?.apiErrorMessage, + "Error: The WooCommerce Shipping & Tax server returned: " + + "Bad Request Unable to request refund. " + + "The parcel has been shipped. ( 400 )") } func test_shippingAddressValidation_returns_address_on_success() throws { diff --git a/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift index 727528eeedc..f51cfe5006c 100644 --- a/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/SiteRemoteTests.swift @@ -50,10 +50,10 @@ final class SiteRemoteTests: XCTestCase { await assertThrowsError({ // When _ = try await remote.createSite(name: "Wapuu swags", flow: .onboarding(domain: "wapuu.store")) - }, errorAssert: { + }, errorAssert: { guard let networkError = $0 as? NetworkError else { return false } return networkError.apiErrorCode == "blog_name_only_lowercase_letters_and_numbers" && - networkError.apiErrorMessage == "Site names can only contain lowercase letters (a-z) and numbers." + networkError.apiErrorMessage == "Site names can only contain lowercase letters (a-z) and numbers." }) } @@ -132,7 +132,7 @@ final class SiteRemoteTests: XCTestCase { }, errorAssert: { error in guard let networkError = error as? NetworkError else { return false } return networkError.apiErrorCode == "no-upgrades-permitted" && - networkError.apiErrorMessage == "You cannot add WordPress.com eCommerce Trial when you already have paid upgrades" + networkError.apiErrorMessage == "You cannot add WordPress.com eCommerce Trial when you already have paid upgrades" }) } @@ -321,10 +321,10 @@ final class SiteRemoteTests: XCTestCase { await assertThrowsError({ // When _ = try await remote.loadSite(domain: domain) - }, errorAssert: { + }, errorAssert: { guard let networkError = $0 as? NetworkError else { return false } return networkError.apiErrorCode == "parse_error" && - networkError.apiErrorMessage == "The Jetpack site is inaccessible or returned an error: parse error (local). not well formed [-32710]" + networkError.apiErrorMessage == "The Jetpack site is inaccessible or returned an error: parse error (local). not well formed [-32710]" }) } From 5d1374bf6376acb6e572f33049ef4f115d01b697 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 2 Sep 2025 15:50:39 +0100 Subject: [PATCH 19/19] Remove unused code --- .../NetworkingCore/Model/DotcomError.swift | 6 +++--- .../NetworkingCore/Network/NetworkError.swift | 16 ++++++++-------- .../Tools/CommonReaderConfigProvider.swift | 15 --------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/Modules/Sources/NetworkingCore/Model/DotcomError.swift b/Modules/Sources/NetworkingCore/Model/DotcomError.swift index 7b11a3df81d..1853e42176d 100644 --- a/Modules/Sources/NetworkingCore/Model/DotcomError.swift +++ b/Modules/Sources/NetworkingCore/Model/DotcomError.swift @@ -177,11 +177,11 @@ public extension NetworkError { guard let originalNetworkError = originalNetworkError else { // No original NetworkError - this is likely from successful HTTP response with API error content let (code, message) = dotcomError.getCodeAndMessage() - let errorData = dotcomError.createEnhancedErrorResponseData(originalResponse: nil) + let errorData = dotcomError.createEnhancedErrorResponseData() return .apiError(code: code, message: message, response: errorData) } - let enhancedErrorData = dotcomError.createEnhancedErrorResponseData(originalResponse: originalNetworkError.response) + let enhancedErrorData = dotcomError.createEnhancedErrorResponseData() switch originalNetworkError { case .notFound: @@ -205,7 +205,7 @@ public extension NetworkError { // private extension DotcomError { /// Creates enhanced JSON error response data that preserves original response while adding structured error details - func createEnhancedErrorResponseData(originalResponse: Data?) -> Data? { + func createEnhancedErrorResponseData() -> Data? { let (code, message) = getCodeAndMessage() let errorResponse: [String: Any] = [ diff --git a/Modules/Sources/NetworkingCore/Network/NetworkError.swift b/Modules/Sources/NetworkingCore/Network/NetworkError.swift index cd9e531c1a7..78c3e713adf 100644 --- a/Modules/Sources/NetworkingCore/Network/NetworkError.swift +++ b/Modules/Sources/NetworkingCore/Network/NetworkError.swift @@ -164,6 +164,7 @@ extension NetworkError: CustomStringConvertible { public extension NetworkError { /// Returns true if this error represents a "not found" condition + // periphery:ignore - TODO: remove this ignore when we merge NetworkError use var isNotFound: Bool { switch self { case .notFound: @@ -176,6 +177,7 @@ public extension NetworkError { } /// Returns true if this error represents an authorization issue + // periphery:ignore - TODO: remove this ignore when we merge NetworkError use var isUnauthorized: Bool { switch self { case .invalidCookieNonce: @@ -190,6 +192,7 @@ public extension NetworkError { } /// Returns true if this error represents a timeout + // periphery:ignore - TODO: remove this ignore when we merge NetworkError use var isTimeout: Bool { switch self { case .timeout: @@ -202,6 +205,7 @@ public extension NetworkError { } /// Returns true if this error represents invalid input/parameters + // periphery:ignore - TODO: remove this ignore when we merge NetworkError use var isInvalidInput: Bool { switch self { case .unacceptableStatusCode(let statusCode, _): @@ -212,6 +216,7 @@ public extension NetworkError { } /// Returns a user-friendly error message, preferring API error message over generic description + // periphery:ignore - TODO: remove this ignore when we merge NetworkError use var userFriendlyMessage: String { return apiErrorMessage ?? localizedDescription } @@ -220,12 +225,7 @@ public extension NetworkError { // MARK: - Supporting Types /// Represents error details from API response JSON -public struct APIErrorDetails: Codable { - public let code: String - public let message: String? - - public init(code: String, message: String?) { - self.code = code - self.message = message - } +struct APIErrorDetails: Codable { + let code: String + let message: String? } diff --git a/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift b/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift index 5c1cc407fef..c16b248b66a 100644 --- a/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift +++ b/Modules/Sources/Yosemite/Tools/CommonReaderConfigProvider.swift @@ -83,18 +83,3 @@ private extension CardReaderConfigError { } } } - -private struct CardReaderConfigNetworkErrorDetails: Decodable { - let code: ErrorCode - let message: String? - - enum CodingKeys: CodingKey { - case code - case message - } - - enum ErrorCode: String, Decodable { - case storeAddressIncomplete = "store_address_is_incomplete" - case postalCodeInvalid = "postal_code_invalid" - } -}