Refactor local exceptions in Swift #2378
Conversation
| @@ -14,6 +14,12 @@ toNSString(const std::string& s) | |||
| return [[NSString alloc] initWithUTF8String:s.c_str()]; | |||
| } | |||
|
|
|||
| inline NSString* | |||
| toNSString(const char* s) | |||
There was a problem hiding this comment.
No need to do const char* -> std::string -> NSString like we did previously.
| #import "IceUtil.h" | ||
| #import "ImplicitContext.h" | ||
| #import "LocalExceptionFactory.h" |
There was a problem hiding this comment.
Renamed Exception.h -> LocalExceptionFactory.h
| category: String, | ||
| facet: String, | ||
| operation: String, file: String, line: Int32 | ||
| let className = typeId.dropFirst(2).replacingOccurrences(of: "::", with: ".") |
There was a problem hiding this comment.
Same logic as for localException below.
|
|
||
| static func protocolException(_ reason: String, file: String, line: Int32) -> Error { | ||
| return ProtocolException(reason: reason, file: file, line: line) | ||
| let className = typeId.dropFirst(2).replacingOccurrences(of: "::", with: ".") |
There was a problem hiding this comment.
Generate fully qualified class name from Slice type ID.
| return if let localExceptionType = NSClassFromString(className) as? LocalException.Type { | ||
| localExceptionType.init(message: message, cxxDescription: cxxDescription, file: file, line: line) | ||
| } else { | ||
| CxxLocalException(typeId: typeId, message: message, cxxDescription: cxxDescription, file: file, line: line) |
There was a problem hiding this comment.
Used for C++ local exceptions not mapped to Swift exceptions such as Ice::FileException.
| /// - cxxDescription: The C++ exception description. | ||
| /// - file: The file where the exception was thrown. | ||
| /// - line: The line where the exception was thrown. | ||
| public required init(message: String, cxxDescription: String, file: String, line: Int32) { |
There was a problem hiding this comment.
"required" is required for creating instances by name (see NSClassFromString code).
| self.cxxDescription = cxxDescription | ||
| } | ||
|
|
||
| public func ice_id() -> String { |
There was a problem hiding this comment.
Need a "virtual" (overridable) ice_id, so can't use an extension method.
| @@ -8,10 +8,7 @@ class PropertiesI: LocalObject<ICEProperties>, Properties { | |||
| } | |||
|
|
|||
| public func getIceProperty(_ key: String) throws -> String { | |||
| guard let value = handle.getIceProperty(key) else { | |||
There was a problem hiding this comment.
It's more correct to throw the exception in the Objective-C++ code.
| @@ -0,0 +1,35 @@ | |||
| // Copyright (c) ZeroC, Inc. | |||
|
|
|||
| /// Base class for Ice user exceptions. | |||
There was a problem hiding this comment.
This PR does not update user exceptions.
| override open func ice_print() -> String { | ||
| return _FixedProxyExceptionDescription | ||
| /// Base class for Ice local exceptions. | ||
| open class LocalException: Exception, CustomStringConvertible { |
There was a problem hiding this comment.
Should it be public class instead?
I kept open class since that what we have in 3.7.
There was a problem hiding this comment.
public class seems more correct now. I suspect we needed open in 3.7 for local slice.
| /// - badTypeId: The type ID of the user exception carried by the reply. | ||
| /// - file: The file where the exception was thrown. | ||
| /// - line: The line where the exception was thrown. | ||
| public convenience init(badTypeId: String, file: String = #fileID, line: Int32 = #line) { |
There was a problem hiding this comment.
is it correct to use #fileID as a default, won't this give us the LocalExceptions.swift file ID?
There was a problem hiding this comment.
No, this works fine in Swift.
Co-authored-by: Jose <[email protected]>
Co-authored-by: Jose <[email protected]>
Co-authored-by: Jose <[email protected]>
| override open func ice_print() -> String { | ||
| return _FixedProxyExceptionDescription | ||
| /// Base class for Ice local exceptions. | ||
| open class LocalException: Exception, CustomStringConvertible { |
There was a problem hiding this comment.
public class seems more correct now. I suspect we needed open in 3.7 for local slice.
The PR refactors the Ice local exceptions in Swift.
In particular: