review comments

Author: rnro

Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift

@@ -281,7 +281,7 @@ final class HTTP1ClientChannelHandler: ChannelDuplexHandler {
         case .close:
             context.close(promise: nil)
 
-        case .wait, .noAction:
+        case .wait:
             break
 
         case .forwardResponseHead(let head, let pauseRequestBodyStream):

Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ConnectionStateMachine.swift

@@ -83,8 +83,6 @@ struct HTTP1ConnectionStateMachine {
         case fireChannelActive
         case fireChannelInactive
         case fireChannelError(Error, closeConnection: Bool)
-
-        case noAction
     }
 
     private var state: State
@@ -361,7 +359,7 @@ struct HTTP1ConnectionStateMachine {
 
     mutating func idleWriteTimeoutTriggered() -> Action {
         guard case .inRequest(var requestStateMachine, let close) = self.state else {
-            return .noAction
+            return .wait
         }
 
         return self.avoidingStateMachineCoW { state -> Action in

Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift

@@ -841,17 +841,6 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
         XCTAssertEqual(request.events.map(\.kind), [.willExecuteRequest, .requestHeadSent])
     }
 
-    class SlowHandler: ChannelOutboundHandler {
-        typealias OutboundIn = HTTPClientRequestPart
-        typealias OutboundOut = HTTPClientRequestPart
-
-        func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
-            context.eventLoop.scheduleTask(in: .milliseconds(300)) {
-                promise?.succeed()
-            }
-        }
-    }
-
     func testIdleWriteTimeoutOutsideOfRunningState() {
         let embedded = EmbeddedChannel()
         var maybeTestUtils: HTTP1TestTools?
@@ -870,8 +859,8 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
         }
         request.body = .init(contentLength: nil, stream: streamCallback)
 
-        let delegate = NullResponseDelegate()
-        var maybeRequestBag: RequestBag<NullResponseDelegate>?
+        let accumulator = ResponseAccumulator(request: request)
+        var maybeRequestBag: RequestBag<ResponseAccumulator>?
         XCTAssertNoThrow(
             maybeRequestBag = try RequestBag(
                 request: request,
@@ -883,7 +872,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
                     idleReadTimeout: .milliseconds(10),
                     idleWriteTimeout: .milliseconds(2)
                 ),
-                delegate: delegate
+                delegate: accumulator
             )
         )
         guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
@@ -908,15 +897,6 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
     }
 }
 
-class NullResponseDelegate: HTTPClientResponseDelegate {
-    typealias Response = Void
-
-    func didFinishRequest(task: AsyncHTTPClient.HTTPClient.Task<Void>) throws {
-        ()
-    }
-
-}
-
 class TestBackpressureWriter {
     let eventLoop: EventLoop
 

Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift

@@ -101,6 +101,26 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
         XCTAssertEqual(state.read(), .read)
     }
 
+    func testWriteTimeoutAfterErrorDoesntCrash() {
+        var state = HTTP1ConnectionStateMachine()
+        XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
+
+        let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
+        let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
+        let newRequestAction = state.runNewRequest(head: requestHead, metadata: metadata)
+        XCTAssertEqual(newRequestAction, .sendRequestHead(requestHead, sendEnd: true))
+        XCTAssertEqual(
+            state.headSent(),
+            .notifyRequestHeadSendSuccessfully(resumeRequestBodyStream: false, startIdleTimer: true)
+        )
+
+        struct MyError: Error, Equatable {}
+        XCTAssertEqual(state.errorHappened(MyError()), .failRequest(MyError(), .close(nil)))
+
+        // Primarily we care that we don't crash here
+        XCTAssertEqual(state.idleWriteTimeoutTriggered(), .wait)
+    }
+
     func testAConnectionCloseHeaderInTheRequestLeadsToConnectionCloseAfterRequest() {
         var state = HTTP1ConnectionStateMachine()
         XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)