Fix last peer initiated stream ID when quiescing (#1700)

Motivation:

The idle handler records the ID of the last stream created by the remote
peer and uses it when sending GOAWAY frames. In most paths this was
correctly updated according to the role the handler played in the
connection and the stream ID. However, in the quiescing state it was
unconditionally updated. This can lead to cases where the client
attempts to send a GOAWAY with a client initiated stream ID: this is
invalid and NIO HTTP/2 treats it as a connection level error, closing
all open streams.

Modification:

- Conditionally update the last peer initiated stream ID when quiescing

Result:

Fewer connection errors

Author: glbrntt

Sources/GRPC/GRPCIdleHandlerStateMachine.swift

@@ -275,7 +275,15 @@ struct GRPCIdleHandlerStateMachine {
       operations.cancelIdleTask(state.idleTask)
 
     case var .quiescing(state):
-      state.lastPeerInitiatedStreamID = streamID
+      switch state.role {
+      case .client where streamID.isServerInitiated:
+        state.lastPeerInitiatedStreamID = streamID
+      case .server where streamID.isClientInitiated:
+        state.lastPeerInitiatedStreamID = streamID
+      default:
+        ()
+      }
+
       state.openStreams += 1
       self.state = .quiescing(state)
 

Tests/GRPCTests/GRPCIdleHandlerStateMachineTests.swift

@@ -535,6 +535,34 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
     _ = stateMachine.channelInactive()
     stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()
   }
+
+  func testStreamIDWhenQuiescing() {
+    var stateMachine = self.makeClientStateMachine()
+    let op1 = stateMachine.receiveSettings([])
+    op1.assertConnectionManager(.ready)
+
+    // Open a stream so we enter quiescing when receiving the GOAWAY.
+    let op2 = stateMachine.streamCreated(withID: 1)
+    op2.assertDoNothing()
+
+    let op3 = stateMachine.receiveGoAway()
+    op3.assertConnectionManager(.quiescing)
+
+    // Create a new stream. This can happen if the GOAWAY races with opening the stream; HTTP2 will
+    // open and then close the stream with an error.
+    let op4 = stateMachine.streamCreated(withID: 3)
+    op4.assertDoNothing()
+
+    // Close the newly opened stream.
+    let op5 = stateMachine.streamClosed(withID: 3)
+    op5.assertDoNothing()
+
+    // Close the original stream.
+    let op6 = stateMachine.streamClosed(withID: 1)
+    // Now we can send a GOAWAY with stream ID zero (we're the client and the server didn't open
+    // any streams).
+    XCTAssertEqual(op6.sendGoAwayWithLastPeerInitiatedStreamID, 0)
+  }
 }
 
 extension GRPCIdleHandlerStateMachine.Operations {