-
Notifications
You must be signed in to change notification settings - Fork 16
Make WordPressOrgXMLRPCApiError conform to CustomNSError
#790
base: trunk
Are you sure you want to change the base?
Conversation
| domain: WordPressOrgXMLRPCApiErrorDomain, | ||
| code: code, | ||
| userInfo: userInfo | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you create a WordPressOrgXMLRPCApiError error here, cast it to NSError, and return it to Objective-C code, the instance can be cast back to WordPressOrgXMLRPCApiError in Swift code when the error is passed from Objective-C code to Swift.
I don't suppose you can cast this NSError instance to WordPressOrgXMLRPCApiError? It would be pretty magical if you can. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is a shortsighted leftover from before I realized that the error domain constant could be defined in APIInterface and therefore be available to packages in both Objective-C and Swift.
I shall remove this utils like I did for WordPressComRestApiErrorDomain.
This ensures that the error domain is the same between SPM and Framework/CocoaPods builds.
Jumping through this hoop was necessary to avoid the Swift compiler automatically generating an error domain for the `Error` and making it impossible to successfully redefine the domain at the `CustomNSError` conformance site.
22dcdd8 to
ac15e9a
Compare
| public struct WordPressOrgXMLRPCApiError: Error { | ||
|
|
||
| /// Error constants for the WordPress XML-RPC API | ||
| @objc public enum Code: Int, CaseIterable { | ||
| /// An error HTTP status code was returned. | ||
| case httpErrorStatusCode | ||
| /// The serialization of the request failed. | ||
| case requestSerializationFailed | ||
| /// The serialization of the response failed. | ||
| case responseSerializationFailed | ||
| /// An unknown error occurred. | ||
| case unknown | ||
| } | ||
|
|
||
| let code: Code | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazytonyli I ended up having to wrap the error code enum into an Error type like you did for the REST API error.
Otherwise, between the @objc visibility (needed because WordPress-Authenticator uses it 🙄 ) and the Error conformance, I couldn't find a way to stop the compiler from automatically generating WordPressOrgXMLRPCApiErrorDomain for @objc public enum WordPressOrgXMLRPCApiError: Int, Error.
I think this is an okay solution. Apart for the fact that's the only one I could get to work, it's, as far as I can see, the same setup as the other error, which makes the implementation consistent.
| failure: { (error, _) in | ||
| expect.fulfill() | ||
|
|
||
| XCTAssertTrue(error is WordPressOrgXMLRPCApiError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only change in behavior from the rewrite.
The NSError-converted error (via asNSerror()) that the API passes to this failure branch doesn't convert back to WordPressOrgXMLRPCApiError.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might have missed something. Does that mean the error in the failure block was WordPressOrgXMLRPCApiError, but in this PR it's changed to something else?
See discussion at #782 (comment)
CHANGELOG.mdif necessary.