Reunify errors into a single enum. (#690)

Motivation:

If we're going to have errors in enumerations, we should aim to have as
few as possible. We wanted to throw away these two, so let's do it.

Modifications:

- Removed `ChannelLifecycleError`, moved case to `ChannelError`.
- Removed `MulticastError`, moved cases to `ChannelError`.
- Removed manual `Equatable` conformance, used synthesised one.
- Pulled `connectFailed` out of `ChannelError`, as it made it impossible to
    write a correct implementation of `Equatable`.
- Conformed `NIOConnectionError` to `Error` instead.

Result:

Better error handling.

Resolves #620

Sources/NIO/BaseSocketChannel.swift

@@ -572,7 +572,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
             return
         }
         guard self.lifecycleManager.isPreRegistered else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
 
@@ -592,7 +592,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
         }
 
         guard self.lifecycleManager.isActive else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
 
@@ -775,7 +775,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
         }
 
         guard !self.lifecycleManager.isPreRegistered else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
 
@@ -1045,7 +1045,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
         }
 
         guard self.lifecycleManager.isPreRegistered else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
 

Sources/NIO/Channel.swift

@@ -296,9 +296,6 @@ public enum ChannelError: Error {
     /// Connect operation timed out
     case connectTimeout(TimeAmount)
 
-    /// Connect operation failed
-    case connectFailed(NIOConnectionError)
-
     /// Unsupported operation triggered on a `Channel`. For example `connect` on a `ServerSocketChannel`.
     case operationUnsupported
 
@@ -328,18 +325,7 @@ public enum ChannelError: Error {
 
     /// A `DatagramChannel` `write` was made with an address that was not reachable and so could not be delivered.
     case writeHostUnreachable
-}
 
-/// This should be inside of `ChannelError` but we keep it separate to not break API.
-// TODO: For 2.0: bring this inside of `ChannelError`. https://github.com/apple/swift-nio/issues/620
-public enum ChannelLifecycleError: Error {
-    /// An operation that was inappropriate given the current `Channel` state was attempted.
-    case inappropriateOperationForState
-}
-
-/// This should be inside of `ChannelError` but we keep it separate to not break API.
-// TODO: For 2.0: bring this inside of `ChannelError`. https://github.com/apple/swift-nio/issues/620
-public enum MulticastError: Error {
     /// The local address of the `Channel` could not be determined.
     case unknownLocalAddress
 
@@ -352,38 +338,12 @@ public enum MulticastError: Error {
     /// An attempt was made to join a multicast group that does not correspond to a multicast
     /// address.
     case illegalMulticastAddress(SocketAddress)
+
+    /// An operation that was inappropriate given the current `Channel` state was attempted.
+    case inappropriateOperationForState
 }
 
-extension ChannelError: Equatable {
-    public static func ==(lhs: ChannelError, rhs: ChannelError) -> Bool {
-        switch (lhs, rhs) {
-        case (.connectPending, .connectPending):
-            return true
-        case (.connectTimeout, .connectTimeout):
-            return true
-        case (.operationUnsupported, .operationUnsupported):
-            return true
-        case (.ioOnClosedChannel, .ioOnClosedChannel):
-            return true
-        case (.alreadyClosed, .alreadyClosed):
-            return true
-        case (.outputClosed, .outputClosed):
-            return true
-        case (.inputClosed, .inputClosed):
-            return true
-        case (.eof, .eof):
-            return true
-        case (.writeDataUnsupported, .writeDataUnsupported):
-            return true
-        case (.writeMessageTooLarge, .writeMessageTooLarge):
-            return true
-        case (.writeHostUnreachable, .writeHostUnreachable):
-            return true
-        default:
-            return false
-        }
-    }
-}
+extension ChannelError: Equatable { }
 
 /// An `Channel` related event that is passed through the `ChannelPipeline` to notify the user.
 public enum ChannelEvent: Equatable {

Sources/NIO/HappyEyeballs.swift

@@ -46,7 +46,7 @@ public struct SingleConnectionFailure {
 
 /// A representation of all the errors that happened during an attempt to connect
 /// to a given host and port.
-public struct NIOConnectionError {
+public struct NIOConnectionError: Error {
     /// The hostname SwiftNIO was trying to connect to.
     public let host: String
 
@@ -518,7 +518,7 @@ internal class HappyEyeballsConnector {
     private func failed() {
         precondition(pendingConnections.count == 0, "failed with pending connections")
         cleanUp()
-        self.resolutionPromise.fail(error: ChannelError.connectFailed(self.error))
+        self.resolutionPromise.fail(error: self.error)
     }
 
     /// Called to connect to a given target.

Sources/NIO/SocketChannel.swift

@@ -377,7 +377,7 @@ final class ServerSocketChannel: BaseSocketChannel<ServerSocket> {
         }
 
         guard self.isRegistered else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
 
@@ -698,7 +698,7 @@ final class DatagramChannel: BaseSocketChannel<Socket> {
     override func bind0(to address: SocketAddress, promise: EventLoopPromise<Void>?) {
         self.eventLoop.assertInEventLoop()
         guard self.isRegistered else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
         do {
@@ -793,25 +793,25 @@ extension DatagramChannel: MulticastChannel {
         self.eventLoop.assertInEventLoop()
 
         guard self.isActive else {
-            promise?.fail(error: ChannelLifecycleError.inappropriateOperationForState)
+            promise?.fail(error: ChannelError.inappropriateOperationForState)
             return
         }
 
         // We need to check that we have the appropriate address types in all cases. They all need to overlap with
         // the address type of this channel, or this cannot work.
         guard let localAddress = self.localAddress else {
-            promise?.fail(error: MulticastError.unknownLocalAddress)
+            promise?.fail(error: ChannelError.unknownLocalAddress)
             return
         }
 
         guard localAddress.protocolFamily == group.protocolFamily else {
-            promise?.fail(error: MulticastError.badMulticastGroupAddressFamily)
+            promise?.fail(error: ChannelError.badMulticastGroupAddressFamily)
             return
         }
 
         // Ok, now we need to check that the group we've been asked to join is actually a multicast group.
         guard group.isMulticast else {
-            promise?.fail(error: MulticastError.illegalMulticastAddress(group))
+            promise?.fail(error: ChannelError.illegalMulticastAddress(group))
             return
         }
 
@@ -838,7 +838,7 @@ extension DatagramChannel: MulticastChannel {
                 try self.socket.setOption(level: CInt(IPPROTO_IPV6), name: operation.optionName(level: CInt(IPPROTO_IPV6)), value: multicastRequest)
             case (.v4, .some(.v6)), (.v6, .some(.v4)), (.v4, .some(.unixDomainSocket)), (.v6, .some(.unixDomainSocket)):
                 // Mismatched group and interface address: this is an error.
-                throw MulticastError.badInterfaceAddressFamily
+                throw ChannelError.badInterfaceAddressFamily
             }
 
             promise?.succeed(result: ())

Tests/NIOTests/ChannelTests.swift

@@ -1983,7 +1983,7 @@ public class ChannelTests: XCTestCase {
             do {
                 try body()
                 XCTFail("didn't throw", file: file, line: line)
-            } catch let error as ChannelLifecycleError where error == .inappropriateOperationForState {
+            } catch let error as ChannelError where error == .inappropriateOperationForState {
                 //OK
             } catch {
                 XCTFail("unexpected error \(error)", file: file, line: line)

Tests/NIOTests/HappyEyeballsTest.swift

@@ -522,12 +522,12 @@ public class HappyEyeballsTest : XCTestCase {
         XCTAssertEqual(resolver.events, expectedQueries)
 
         // But we should have failed.
-        if case .some(ChannelError.connectFailed(let inner)) = channelFuture.getError() {
-            XCTAssertEqual(inner.host, "example.com")
-            XCTAssertEqual(inner.port, 80)
-            XCTAssertNil(inner.dnsAError)
-            XCTAssertNil(inner.dnsAAAAError)
-            XCTAssertEqual(inner.connectionErrors.count, 0)
+        if let error = channelFuture.getError() as? NIOConnectionError {
+            XCTAssertEqual(error.host, "example.com")
+            XCTAssertEqual(error.port, 80)
+            XCTAssertNil(error.dnsAError)
+            XCTAssertNil(error.dnsAAAAError)
+            XCTAssertEqual(error.connectionErrors.count, 0)
         } else {
             XCTFail("Got \(String(describing: channelFuture.getError()))")
         }
@@ -554,12 +554,12 @@ public class HappyEyeballsTest : XCTestCase {
         XCTAssertEqual(resolver.events, expectedQueries)
 
         // But we should have failed.
-        if case .some(ChannelError.connectFailed(let inner)) = channelFuture.getError() {
-            XCTAssertEqual(inner.host, "example.com")
-            XCTAssertEqual(inner.port, 80)
-            XCTAssertEqual(inner.dnsAError as? DummyError ?? DummyError(), v4Error)
-            XCTAssertEqual(inner.dnsAAAAError as? DummyError ?? DummyError(), v6Error)
-            XCTAssertEqual(inner.connectionErrors.count, 0)
+        if let error = channelFuture.getError() as? NIOConnectionError {
+            XCTAssertEqual(error.host, "example.com")
+            XCTAssertEqual(error.port, 80)
+            XCTAssertEqual(error.dnsAError as? DummyError ?? DummyError(), v4Error)
+            XCTAssertEqual(error.dnsAAAAError as? DummyError ?? DummyError(), v6Error)
+            XCTAssertEqual(error.connectionErrors.count, 0)
         } else {
             XCTFail("Got \(String(describing: channelFuture.getError()))")
         }
@@ -689,14 +689,14 @@ public class HappyEyeballsTest : XCTestCase {
         XCTAssertTrue(channelFuture.isFulfilled)
 
         // Check the error.
-        if case .some(ChannelError.connectFailed(let inner)) = channelFuture.getError() {
-            XCTAssertEqual(inner.host, "example.com")
-            XCTAssertEqual(inner.port, 80)
-            XCTAssertNil(inner.dnsAError)
-            XCTAssertNil(inner.dnsAAAAError)
-            XCTAssertEqual(inner.connectionErrors.count, 20)
+        if let error = channelFuture.getError() as? NIOConnectionError {
+            XCTAssertEqual(error.host, "example.com")
+            XCTAssertEqual(error.port, 80)
+            XCTAssertNil(error.dnsAError)
+            XCTAssertNil(error.dnsAAAAError)
+            XCTAssertEqual(error.connectionErrors.count, 20)
 
-            for (idx, error) in inner.connectionErrors.enumerated() {
+            for (idx, error) in error.connectionErrors.enumerated() {
                 XCTAssertEqual(error.error as? DummyError, errors[idx])
             }
         } else {
@@ -1021,7 +1021,7 @@ public class HappyEyeballsTest : XCTestCase {
 
         XCTAssertTrue(channelFuture.isFulfilled)
         switch channelFuture.getError() {
-        case .some(ChannelError.connectFailed):
+        case is NIOConnectionError:
             break
         default:
             XCTFail("Got unexpected error: \(String(describing: channelFuture.getError()))")
@@ -1141,8 +1141,8 @@ public class HappyEyeballsTest : XCTestCase {
         XCTAssertEqual(errors.count, 20)
 
         XCTAssertTrue(channelFuture.isFulfilled)
-        if case .some(ChannelError.connectFailed(let inner)) = channelFuture.getError() {
-            XCTAssertEqual(inner.connectionErrors.map { $0.error as! DummyError }, errors)
+        if let error = channelFuture.getError() as? NIOConnectionError {
+            XCTAssertEqual(error.connectionErrors.map { $0.error as! DummyError }, errors)
         } else {
             XCTFail("Got unexpected error: \(String(describing: channelFuture.getError()))")
         }

docs/public-api-changes-NIO1-to-NIO2.md

@@ -16,3 +16,6 @@
 - changed `HTTPVersion`'s `major` and `minor` properties to `Int` (from `UInt16`)
 - renamed the generic parameter name to `Bytes` where we're talking about a
   generic collection of bytes
+- Moved error `ChannelLifecycleError.inappropriateOperationForState` to `ChannelError.inappropriateOperationForState`.
+- Moved all errors in `MulticastError` enum into `ChannelError`.
+- Removed `ChannelError.connectFailed`. All errors that triggered this now throw `NIOConnectError` directly.