Trailers sent on idle streams deserve to be sent too. (#25)

Motivation:

Due to nghttp2's weird data provider model, all trailers go through a
similar send path as data frames. This includes the fact that the data
sender can go "idle". If the data sender is idle, and the user sends
trailers, we don't bother to tell nghttp2 to ask us for more data,
so the trailers sit there forever, feeling sad.

Modifications:

- Correctly restart the data provider when sending data.

Result:

Trailers will be emitted when the stream is idle.

Author: Lukasa

Sources/NIOHTTP2/NGHTTP2Session.swift

@@ -693,6 +693,14 @@ class NGHTTP2Session {
             // TODO(cory): Error handling.
             precondition(isEndStream)
             streamData.dataProvider.bufferEOF(trailers: headers)
+
+            if case .pending = streamData.dataProvider.state {
+                // The data provider is currently in the pending state, we need to tell nghttp2 it's active again so
+                // the trailers get emitted.
+                let rc = nghttp2_session_resume_data(self.session, frame.streamID.networkStreamID!)
+                precondition(rc == 0)
+                streamData.dataProvider.didResume()
+            }
             return
         }
 

Tests/NIOHTTP2Tests/SimpleClientServerTests+XCTest.swift

@@ -48,6 +48,7 @@ extension SimpleClientServerTests {
                 ("testFailingUnflushedWritesForGoaway", testFailingUnflushedWritesForGoaway),
                 ("testFailingFlushedWritesForGoaway", testFailingFlushedWritesForGoaway),
                 ("testSendingTrailers", testSendingTrailers),
+                ("testSendingTrailersWhenDataProviderIsIdle", testSendingTrailersWhenDataProviderIsIdle),
                 ("test1XXResponseHeaderFields", test1XXResponseHeaderFields),
                 ("testPriorityFramesAreNotEmitted", testPriorityFramesAreNotEmitted),
                 ("testStreamClosedWithNoError", testStreamClosedWithNoError),

Tests/NIOHTTP2Tests/SimpleClientServerTests.swift

@@ -1006,6 +1006,41 @@ class SimpleClientServerTests: XCTestCase {
         XCTAssertNoThrow(try self.serverChannel.finish())
     }
 
+    func testSendingTrailersWhenDataProviderIsIdle() throws {
+        // Begin by getting the connection up.
+        try self.basicHTTP2Connection()
+
+        // We're now going to try to send a request from the client to the server. This request will be one headers frame, one
+        // data frame, and then another headers frame for trailers. The headers frame for trailers will be sent late, to
+        // reproduce https://github.com/apple/swift-nio-http2/issues/24.
+        let headers = HTTPHeaders([(":path", "/"), (":method", "POST"), (":scheme", "https"), (":authority", "localhost")])
+        let trailers = HTTPHeaders([("x-trailers-field", "true")])
+        var requestBody = self.clientChannel.allocator.buffer(capacity: 128)
+        requestBody.write(staticString: "A simple HTTP/2 request.")
+
+        let clientStreamID = HTTP2StreamID()
+        let reqFrame = HTTP2Frame(streamID: clientStreamID, payload: .headers(headers))
+        let reqBodyFrame = HTTP2Frame(streamID: clientStreamID, payload: .data(.byteBuffer(requestBody)))
+        var trailerFrame = HTTP2Frame(streamID: clientStreamID, payload: .headers(trailers))
+        trailerFrame.flags.insert(.endStream)
+
+        let serverStreamID = try self.assertFramesRoundTrip(frames: [reqFrame, reqBodyFrame], sender: self.clientChannel, receiver: self.serverChannel).first!.streamID
+
+        // Now we can send the next trailers.
+        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: [trailerFrame], sender: self.clientChannel, receiver: self.serverChannel))
+
+        // Let's send a quick response back. This response should also contain trailers.
+        let responseHeaders = HTTPHeaders([(":status", "200"), ("content-length", "0")])
+        let respFrame = HTTP2Frame(streamID: serverStreamID, payload: .headers(responseHeaders))
+        var respTrailersFrame = HTTP2Frame(streamID: serverStreamID, payload: .headers(trailers))
+        respTrailersFrame.flags.insert(.endStream)
+        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: [respFrame], sender: self.serverChannel, receiver: self.clientChannel))
+        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: [respTrailersFrame], sender: self.serverChannel, receiver: self.clientChannel))
+
+        XCTAssertNoThrow(try self.clientChannel.finish())
+        XCTAssertNoThrow(try self.serverChannel.finish())
+    }
+
     func test1XXResponseHeaderFields() throws {
         // Begin by getting the connection up.
         try self.basicHTTP2Connection()

Tests/NIOHTTP2Tests/TestUtilities.swift

@@ -119,7 +119,7 @@ extension XCTestCase {
         var receivedFrames = [HTTP2Frame]()
 
         for frame in frames {
-            let receivedFrame = try receiver.assertReceivedFrame()
+            let receivedFrame = try receiver.assertReceivedFrame(file: file, line: line)
             receivedFrame.assertFrameMatches(this: frame, file: file, line: line)
             receivedFrames.append(receivedFrame)
         }