Tolerate new request after connection error happened (swift-server#688)

* Tolerate new request after connection error happened

* fix tests

Author: dnadoba

Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ConnectionStateMachine.swift

@@ -106,14 +106,14 @@ struct HTTP1ConnectionStateMachine {
             return .wait
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
     mutating func channelInactive() -> Action {
         switch self.state {
         case .initialized:
-            preconditionFailure("A channel that isn't active, must not become inactive")
+            fatalError("A channel that isn't active, must not become inactive")
 
         case .inRequest(var requestStateMachine, close: _):
             return self.avoidingStateMachineCoW { state -> Action in
@@ -130,7 +130,7 @@ struct HTTP1ConnectionStateMachine {
             return .wait
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
@@ -155,7 +155,7 @@ struct HTTP1ConnectionStateMachine {
             return .fireChannelError(error, closeConnection: false)
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
@@ -173,7 +173,7 @@ struct HTTP1ConnectionStateMachine {
             }
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
@@ -182,15 +182,15 @@ struct HTTP1ConnectionStateMachine {
         metadata: RequestFramingMetadata
     ) -> Action {
         switch self.state {
-        case .initialized, .closing, .inRequest:
+        case .initialized, .inRequest:
             // These states are unreachable as the connection pool state machine has put the
             // connection into these states. In other words the connection pool state machine must
             // be aware about these states before the connection itself. For this reason the
             // connection pool state machine must not send a new request to the connection, if the
             // connection is `.initialized`, `.closing` or `.inRequest`
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
 
-        case .closed:
+        case .closing, .closed:
             // The remote may have closed the connection and the connection pool state machine
             // was not updated yet because of a race condition. New request vs. marking connection
             // as closed.
@@ -208,13 +208,13 @@ struct HTTP1ConnectionStateMachine {
             return self.state.modify(with: action)
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
     mutating func requestStreamPartReceived(_ part: IOData, promise: EventLoopPromise<Void>?) -> Action {
         guard case .inRequest(var requestStateMachine, let close) = self.state else {
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
 
         return self.avoidingStateMachineCoW { state -> Action in
@@ -226,7 +226,7 @@ struct HTTP1ConnectionStateMachine {
 
     mutating func requestStreamFinished(promise: EventLoopPromise<Void>?) -> Action {
         guard case .inRequest(var requestStateMachine, let close) = self.state else {
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
 
         return self.avoidingStateMachineCoW { state -> Action in
@@ -239,7 +239,7 @@ struct HTTP1ConnectionStateMachine {
     mutating func requestCancelled(closeConnection: Bool) -> Action {
         switch self.state {
         case .initialized:
-            preconditionFailure("This event must only happen, if the connection is leased. During startup this is impossible. Invalid state: \(self.state)")
+            fatalError("This event must only happen, if the connection is leased. During startup this is impossible. Invalid state: \(self.state)")
 
         case .idle:
             if closeConnection {
@@ -260,7 +260,7 @@ struct HTTP1ConnectionStateMachine {
             return .wait
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
@@ -269,7 +269,7 @@ struct HTTP1ConnectionStateMachine {
     mutating func read() -> Action {
         switch self.state {
         case .initialized:
-            preconditionFailure("Why should we read something, if we are not connected yet")
+            fatalError("Why should we read something, if we are not connected yet")
         case .idle:
             return .read
         case .inRequest(var requestStateMachine, let close):
@@ -284,14 +284,14 @@ struct HTTP1ConnectionStateMachine {
             return .read
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
     mutating func channelRead(_ part: HTTPClientResponsePart) -> Action {
         switch self.state {
         case .initialized, .idle:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
 
         case .inRequest(var requestStateMachine, var close):
             return self.avoidingStateMachineCoW { state -> Action in
@@ -310,7 +310,7 @@ struct HTTP1ConnectionStateMachine {
             return .wait
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
@@ -327,13 +327,13 @@ struct HTTP1ConnectionStateMachine {
             }
 
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 
     mutating func demandMoreResponseBodyParts() -> Action {
         guard case .inRequest(var requestStateMachine, let close) = self.state else {
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
 
         return self.avoidingStateMachineCoW { state -> Action in
@@ -345,7 +345,7 @@ struct HTTP1ConnectionStateMachine {
 
     mutating func idleReadTimeoutTriggered() -> Action {
         guard case .inRequest(var requestStateMachine, let close) = self.state else {
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
 
         return self.avoidingStateMachineCoW { state -> Action in
@@ -423,7 +423,7 @@ extension HTTP1ConnectionStateMachine.State {
             return .forwardResponseBodyParts(parts)
         case .succeedRequest(let finalAction, let finalParts):
             guard case .inRequest(_, close: let close) = self else {
-                preconditionFailure("Invalid state: \(self)")
+                fatalError("Invalid state: \(self)")
             }
 
             let newFinalAction: HTTP1ConnectionStateMachine.Action.FinalSuccessfulStreamAction
@@ -443,9 +443,9 @@ extension HTTP1ConnectionStateMachine.State {
         case .failRequest(let error, let finalAction):
             switch self {
             case .initialized:
-                preconditionFailure("Invalid state: \(self)")
+                fatalError("Invalid state: \(self)")
             case .idle:
-                preconditionFailure("How can we fail a task, if we are idle")
+                fatalError("How can we fail a task, if we are idle")
             case .inRequest(_, close: let close):
                 if case .close(let promise) = finalAction {
                     self = .closing
@@ -465,7 +465,7 @@ extension HTTP1ConnectionStateMachine.State {
                 return .failRequest(error, .none)
 
             case .modifying:
-                preconditionFailure("Invalid state: \(self)")
+                fatalError("Invalid state: \(self)")
             }
 
         case .read:
@@ -497,7 +497,7 @@ extension HTTP1ConnectionStateMachine: CustomStringConvertible {
         case .closed:
             return ".closed"
         case .modifying:
-            preconditionFailure("Invalid state: \(self.state)")
+            fatalError("Invalid state: \(self.state)")
         }
     }
 }

Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift

@@ -200,6 +200,19 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
         XCTAssertEqual(state.requestCancelled(closeConnection: false), .failRequest(HTTPClientError.cancelled, .close(nil)))
     }
 
+    func testNewRequestAfterErrorHappened() {
+        var state = HTTP1ConnectionStateMachine()
+        XCTAssertEqual(state.channelActive(isWritable: false), .fireChannelActive)
+        struct MyError: Error, Equatable {}
+        XCTAssertEqual(state.errorHappened(MyError()), .fireChannelError(MyError(), closeConnection: true))
+        let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"])
+        let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
+        let action = state.runNewRequest(head: requestHead, metadata: metadata)
+        guard case .failRequest = action else {
+            return XCTFail("unexpected action \(action)")
+        }
+    }
+
     func testCancelRequestIsIgnoredWhenConnectionIsIdle() {
         var state = HTTP1ConnectionStateMachine()
         XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
@@ -303,6 +316,8 @@ extension HTTP1ConnectionStateMachine.Action: Equatable {
 
         case (.fireChannelInactive, .fireChannelInactive):
             return true
+        case (.fireChannelError(_, let lhsCloseConnection), .fireChannelError(_, let rhsCloseConnection)):
+            return lhsCloseConnection == rhsCloseConnection
 
         case (.sendRequestHead(let lhsHead, let lhsStartBody), .sendRequestHead(let rhsHead, let rhsStartBody)):
             return lhsHead == rhsHead && lhsStartBody == rhsStartBody
@@ -377,6 +392,7 @@ extension HTTP1ConnectionStateMachine.Action.FinalFailedStreamAction: Equatable
             return lhsPromise?.futureResult == rhsPromise?.futureResult
         case (.none, .none):
             return true
+
         default:
             return false
         }