Make ChannelError and NIOConnectionError conform to CustomStringConvertible #3230
Conversation
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Thanks for opening this @axelandersson -- I left some feedback inline.
Sources/NIOPosix/HappyEyeballs.swift
Outdated
|
|
||
| extension NIOConnectionError: CustomStringConvertible { | ||
| public var description: String { | ||
| if let dnsError = (dnsAError ?? dnsAAAAError) { |
There was a problem hiding this comment.
nit: coding style in NIO is to use explicit self:
| if let dnsError = (dnsAError ?? dnsAAAAError) { | |
| if let dnsError = (self.dnsAError ?? self.dnsAAAAError) { |
There was a problem hiding this comment.
Thanks - updated.
Sources/NIOPosix/HappyEyeballs.swift
Outdated
| extension NIOConnectionError: CustomStringConvertible { | ||
| public var description: String { | ||
| if let dnsError = (dnsAError ?? dnsAAAAError) { | ||
| return "DNS error: \(dnsError.localizedDescription)" |
There was a problem hiding this comment.
Localized description gives bad error messages unless the underlying error is an NSError:
| return "DNS error: \(dnsError.localizedDescription)" | |
| return "DNS error: \(dnsError)" |
There was a problem hiding this comment.
Thanks - updated.
Sources/NIOPosix/HappyEyeballs.swift
Outdated
| return "Connection errors: \(descriptions.joined(separator: ", "))" | ||
| } | ||
|
|
||
| return "Unknown error" |
There was a problem hiding this comment.
I think this can still hint that it's a connection error: e.g. "NIOConnectionError: unknown"
There was a problem hiding this comment.
Thanks - updated.
Sources/NIOPosix/HappyEyeballs.swift
Outdated
| if let channelError = $0.error as? ChannelError { | ||
| channelError.description | ||
| } else { | ||
| $0.error.localizedDescription | ||
| } |
There was a problem hiding this comment.
No need to do any casting here and avoid using localizedDescription. Just use String(describing: $0) here.
There was a problem hiding this comment.
Thanks - updated.
2 participants
Motivation:
ChannelErrorandNIOConnectionErrorshould conformCustomStringConvertibleso that these errors can be easily logged.As discussed in #3222.
Modifications:
Added conformance to
ChannelErrorand wrote error messages for all cases.Added conformance to
NIOConnectionError.