Code review

Author: fabianfett

Diff for: Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift

@@ -24,9 +24,9 @@ class HTTP2ClientRequestHandler: ChannelDuplexHandler {
 
     private let eventLoop: EventLoop
 
-    private(set) var channelContext: ChannelHandlerContext?
-    private(set) var state: HTTPRequestStateMachine = .init(isChannelWritable: false)
-    private(set) var request: HTTPExecutingRequest?
+    private var channelContext: ChannelHandlerContext?
+    private var state: HTTPRequestStateMachine = .init(isChannelWritable: false)
+    private var request: HTTPExecutingRequest?
 
     init(eventLoop: EventLoop) {
         self.eventLoop = eventLoop
@@ -96,10 +96,13 @@ class HTTP2ClientRequestHandler: ChannelDuplexHandler {
         case .sendRequestHead(let head, let startBody):
             if startBody {
                 context.writeAndFlush(self.wrapOutboundOut(.head(head)), promise: nil)
+                self.request!.requestHeadSent()
                 self.request!.resumeRequestBodyStream()
             } else {
-                context.writeAndFlush(self.wrapOutboundOut(.head(head)), promise: nil)
-                context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil)
+                context.write(self.wrapOutboundOut(.head(head)), promise: nil)
+                context.write(self.wrapOutboundOut(.end(nil)), promise: nil)
+                context.flush()
+                self.request!.requestHeadSent()
             }
 
         case .pauseRequestBodyStream:

Diff for: Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2IdleHandler.swift

@@ -120,7 +120,7 @@ extension HTTP2IdleHandler {
                 preconditionFailure("Invalid state")
             }
 
-            let maxStream = settings.first(where: { $0.parameter == .maxConcurrentStreams })?.value ?? 100
+            let maxStream = settings.last(where: { $0.parameter == .maxConcurrentStreams })?.value ?? 100
 
             self.state = .active(openStreams: 0, maxStreams: maxStream)
             return .notifyConnectionNewSettings(settings)
@@ -137,7 +137,7 @@ extension HTTP2IdleHandler {
                 self.state = .goAwayReceived(openStreams: openStreams, maxStreams: maxStreams)
                 return .notifyConnectionGoAwayReceived
             case .goAwayReceived:
-                preconditionFailure("Invalid state")
+                return .nothing
             case .closed:
                 preconditionFailure("Invalid state")
             }

Diff for: Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift

@@ -42,6 +42,7 @@ extension HTTPRequestStateMachineTests {
             ("testResponseReadingWithBackpressureEndOfResponseAllowsReadEventsToTriggerDirectly", testResponseReadingWithBackpressureEndOfResponseAllowsReadEventsToTriggerDirectly),
             ("testCancellingARequestInStateInitializedKeepsTheConnectionAlive", testCancellingARequestInStateInitializedKeepsTheConnectionAlive),
             ("testCancellingARequestBeforeBeingSendKeepsTheConnectionAlive", testCancellingARequestBeforeBeingSendKeepsTheConnectionAlive),
+            ("testConnectionBecomesWritableBeforeFirstRequest", testConnectionBecomesWritableBeforeFirstRequest),
             ("testCancellingARequestThatIsSent", testCancellingARequestThatIsSent),
             ("testRemoteSuddenlyClosesTheConnection", testRemoteSuddenlyClosesTheConnection),
             ("testReadTimeoutLeadsToFailureWithEverythingAfterBeingIgnored", testReadTimeoutLeadsToFailureWithEverythingAfterBeingIgnored),

Diff for: Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift

@@ -350,6 +350,24 @@ class HTTPRequestStateMachineTests: XCTestCase {
         XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
     }
 
+    func testConnectionBecomesWritableBeforeFirstRequest() {
+        var state = HTTPRequestStateMachine(isChannelWritable: false)
+        XCTAssertEqual(state.writabilityChanged(writable: true), .wait)
+
+        // --- sending request
+        let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
+        let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
+        XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
+
+        // --- receiving response
+        let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["content-length": "4"])
+        XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
+        let responseBody = ByteBuffer(bytes: [1, 2, 3, 4])
+        XCTAssertEqual(state.channelRead(.body(responseBody)), .wait)
+        XCTAssertEqual(state.channelRead(.end(nil)), .succeedRequest(.none, .init([responseBody])))
+        XCTAssertEqual(state.channelReadComplete(), .wait)
+    }
+
     func testCancellingARequestThatIsSent() {
         var state = HTTPRequestStateMachine(isChannelWritable: true)
         let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")