From b191974ce7976180c767eecdc386a7202518ca97 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Dec 2023 10:46:14 +1300 Subject: [PATCH 1/4] Fix incorrect error message implementation in WordPressAPIError --- WordPressKit.xcodeproj/project.pbxproj | 4 +++ WordPressKit/WordPressAPIError.swift | 4 +-- .../Utilities/WordPressAPIErrorTests.swift | 26 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 WordPressKitTests/Utilities/WordPressAPIErrorTests.swift diff --git a/WordPressKit.xcodeproj/project.pbxproj b/WordPressKit.xcodeproj/project.pbxproj index 86714efd..26028053 100644 --- a/WordPressKit.xcodeproj/project.pbxproj +++ b/WordPressKit.xcodeproj/project.pbxproj @@ -144,6 +144,7 @@ 4A11239E2B1926D1004690CF /* HTTPClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A11239D2B1926D1004690CF /* HTTPClient.swift */; }; 4A1DEF44293051BC00322608 /* LoggingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A1DEF43293051BC00322608 /* LoggingTests.swift */; }; 4A1DEF46293051C600322608 /* LoggingTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 4A1DEF45293051C600322608 /* LoggingTests.m */; }; + 4A40F6552B2A5A1A0015DA77 /* WordPressAPIErrorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A40F6542B2A5A1A0015DA77 /* WordPressAPIErrorTests.swift */; }; 4A68E3CD29404181004AC3DC /* RemoteBlog.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A68E3CC29404181004AC3DC /* RemoteBlog.swift */; }; 4A68E3CF29404289004AC3DC /* RemoteBlogOptionsHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A68E3CE29404289004AC3DC /* RemoteBlogOptionsHelper.swift */; }; 4A68E3D329406AA0004AC3DC /* RemoteMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A68E3D029406AA0004AC3DC /* RemoteMenu.swift */; }; @@ -840,6 +841,7 @@ 4A11239D2B1926D1004690CF /* HTTPClient.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HTTPClient.swift; sourceTree = ""; }; 4A1DEF43293051BC00322608 /* LoggingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoggingTests.swift; sourceTree = ""; }; 4A1DEF45293051C600322608 /* LoggingTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = LoggingTests.m; sourceTree = ""; }; + 4A40F6542B2A5A1A0015DA77 /* WordPressAPIErrorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WordPressAPIErrorTests.swift; sourceTree = ""; }; 4A68E3CC29404181004AC3DC /* RemoteBlog.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoteBlog.swift; sourceTree = ""; }; 4A68E3CE29404289004AC3DC /* RemoteBlogOptionsHelper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoteBlogOptionsHelper.swift; sourceTree = ""; }; 4A68E3D029406AA0004AC3DC /* RemoteMenu.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoteMenu.swift; sourceTree = ""; }; @@ -2638,6 +2640,7 @@ 4A1DEF45293051C600322608 /* LoggingTests.m */, 4A6B4A832B26974F00802316 /* HTTPRequestBuilderTests.swift */, 4A6B4A852B269D0C00802316 /* URLSessionHelperTests.swift */, + 4A40F6542B2A5A1A0015DA77 /* WordPressAPIErrorTests.swift */, ); path = Utilities; sourceTree = ""; @@ -3502,6 +3505,7 @@ 740B23D61F17F7C100067A2A /* XMLRPCTestable.swift in Sources */, FFE247A720C891D1002DF3A2 /* WordPressComOAuthClientTests.swift in Sources */, 93AB06041EE8838400EF8764 /* RemoteTestCase.swift in Sources */, + 4A40F6552B2A5A1A0015DA77 /* WordPressAPIErrorTests.swift in Sources */, BA2A78FA24A486D300BB6F53 /* SitePluginTests.swift in Sources */, 465F88A7263B371D00F4C950 /* BlockEditorSettingsServiceRemoteTests.swift in Sources */, 93BD27411EE73311002BB00B /* AccountServiceRemoteRESTTests.swift in Sources */, diff --git a/WordPressKit/WordPressAPIError.swift b/WordPressKit/WordPressAPIError.swift index fa15b64a..ffff68a6 100644 --- a/WordPressKit/WordPressAPIError.swift +++ b/WordPressKit/WordPressAPIError.swift @@ -21,7 +21,7 @@ public enum WordPressAPIError: Error where EndpointError: Localiz case unknown(underlyingError: Error) } -extension WordPressComOAuthError: LocalizedError { +extension WordPressAPIError: LocalizedError { public var errorDescription: String? { switch self { @@ -29,7 +29,7 @@ extension WordPressComOAuthError: LocalizedError { // These are usually programming errors. return Self.unknownErrorMessage case let .endpointError(error): - return error.errorDescription + return error.errorDescription ?? Self.unknownErrorMessage case let .connection(error): return error.localizedDescription case let .unknown(underlyingError): diff --git a/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift b/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift new file mode 100644 index 00000000..5afd2687 --- /dev/null +++ b/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift @@ -0,0 +1,26 @@ +import Foundation +import XCTest +@testable import WordPressKit + +class WordPressAPIErrorTests: XCTestCase { + + func testLocalizedMessage() { + struct TestError: LocalizedError { + var errorDescription: String? = "this is a test error" + } + + let error = WordPressAPIError.endpointError(TestError()) + XCTAssertEqual((error as NSError).localizedDescription, "this is a test error") + } + + func testNilErrorDescription() { + struct TestError: LocalizedError { + var errorDescription: String? = nil + } + + let error = WordPressAPIError.endpointError(TestError()) + XCTAssertEqual(error.localizedDescription, WordPressAPIError.unknownErrorMessage) + XCTAssertEqual((error as NSError).localizedDescription, WordPressAPIError.unknownErrorMessage) + } + +} From 77c9d8f3a4369007b332341e8c75f57c44d3909f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Dec 2023 10:50:49 +1300 Subject: [PATCH 2/4] Add a changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 984253ff..c10c778a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ _None._ ### Internal Changes -_None._ +- Fix `WordPressAPIError`'s localized error message. [#662] ## 9.0.0 From 39a67df2ce2d585a5850f1b6ff0ca47fca7d2af0 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Dec 2023 19:01:52 +1300 Subject: [PATCH 3/4] Add another assertion Co-authored-by: Gio Lodi --- WordPressKitTests/Utilities/WordPressAPIErrorTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift b/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift index 5afd2687..2b653f7d 100644 --- a/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift +++ b/WordPressKitTests/Utilities/WordPressAPIErrorTests.swift @@ -10,6 +10,7 @@ class WordPressAPIErrorTests: XCTestCase { } let error = WordPressAPIError.endpointError(TestError()) + XCTAssertEqual(error.errorDescription, "this is a test error") XCTAssertEqual((error as NSError).localizedDescription, "this is a test error") } From 44c6af2d22337e407e1203139de2d94f868b5635 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Dec 2023 19:00:58 +1300 Subject: [PATCH 4/4] Add comment about `errorDescription` should not be nil --- WordPressKit/WordPressAPIError.swift | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/WordPressKit/WordPressAPIError.swift b/WordPressKit/WordPressAPIError.swift index ffff68a6..2e2518cd 100644 --- a/WordPressKit/WordPressAPIError.swift +++ b/WordPressKit/WordPressAPIError.swift @@ -24,20 +24,27 @@ public enum WordPressAPIError: Error where EndpointError: Localiz extension WordPressAPIError: LocalizedError { public var errorDescription: String? { + // Considering `WordPressAPIError` is the error that's surfaced from this library to the apps, its instanes + // may be displayed on UI directly. To prevent Swift's default error message (i.e. "This operation can't be + // completed. (code=...)") from being displayed, we need to make sure this implementation + // always returns a non-nil value. + let localizedErrorMessage: String switch self { case .requestEncodingFailure, .unparsableResponse: // These are usually programming errors. - return Self.unknownErrorMessage + localizedErrorMessage = Self.unknownErrorMessage case let .endpointError(error): - return error.errorDescription ?? Self.unknownErrorMessage + localizedErrorMessage = error.errorDescription ?? Self.unknownErrorMessage case let .connection(error): - return error.localizedDescription + localizedErrorMessage = error.localizedDescription case let .unknown(underlyingError): if let msg = (underlyingError as? LocalizedError)?.errorDescription { - return msg + localizedErrorMessage = msg + } else { + localizedErrorMessage = Self.unknownErrorMessage } - return Self.unknownErrorMessage } + return localizedErrorMessage } }