From 386f6a840e7de43aee5cf95e8beb78ea9e6c8ced Mon Sep 17 00:00:00 2001
From: Matt Joras <mjoras@fb.com>
Date: Mon, 15 Mar 2021 07:47:53 -0700
Subject: [PATCH] Add number of streams to idle timeout error.

Summary: If we idle timeout when there are non control streams open, there could be some sort of issue especially with protocols like HTTP/3.

Reviewed By: yangchi

Differential Revision: D27017861

fbshipit-source-id: 319b839a641fa417a5026adf607c7bd0070cb66c
---
 quic/api/QuicTransportBase.cpp          |  6 +++++-
 quic/api/test/QuicTransportBaseTest.cpp | 26 +++++++++++++++++++++++++
 quic/state/QuicStreamManager.h          |  7 +++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp
index 77fd5a7e1..d1195287f 100644
--- a/quic/api/QuicTransportBase.cpp
+++ b/quic/api/QuicTransportBase.cpp
@@ -2351,10 +2351,14 @@ void QuicTransportBase::idleTimeoutExpired(bool drain) noexcept {
   // idle timeout is expired, just close the connection and drain or
   // send connection close immediately depending on 'drain'
   DCHECK_NE(closeState_, CloseState::CLOSED);
+  uint64_t numOpenStreans = conn_->streamManager->streamCount();
   closeImpl(
       std::make_pair(
           QuicErrorCode(LocalErrorCode::IDLE_TIMEOUT),
-          toString(LocalErrorCode::IDLE_TIMEOUT).str()),
+          folly::to<std::string>(
+              toString(LocalErrorCode::IDLE_TIMEOUT),
+              ", num non control streams: ",
+              numOpenStreans - conn_->streamManager->numControlStreams())),
       drain /* drainConnection */,
       !drain /* sendCloseImmediately */);
 }
diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp
index 5db887ddf..c76f0db65 100644
--- a/quic/api/test/QuicTransportBaseTest.cpp
+++ b/quic/api/test/QuicTransportBaseTest.cpp
@@ -429,6 +429,32 @@ TEST_F(QuicTransportImplTest, IdleTimeoutExpiredDestroysTransport) {
   transport->invokeIdleTimeout();
 }
 
+TEST_F(QuicTransportImplTest, IdleTimeoutStreamMaessage) {
+  auto stream1 = transport->createBidirectionalStream().value();
+  auto stream2 = transport->createBidirectionalStream().value();
+  auto stream3 = transport->createUnidirectionalStream().value();
+  transport->setControlStream(stream3);
+
+  NiceMock<MockReadCallback> readCb1;
+  NiceMock<MockReadCallback> readCb2;
+
+  transport->setReadCallback(stream1, &readCb1);
+  transport->setReadCallback(stream2, &readCb2);
+
+  transport->addDataToStream(
+      stream1, StreamBuffer(folly::IOBuf::copyBuffer("actual stream data"), 0));
+  transport->addDataToStream(
+      stream2,
+      StreamBuffer(folly::IOBuf::copyBuffer("actual stream data"), 10));
+  EXPECT_CALL(readCb1, readError(stream1, _))
+      .Times(1)
+      .WillOnce(Invoke([](auto, auto error) {
+        EXPECT_EQ(
+            "Idle timeout, num non control streams: 2", error.second->str());
+      }));
+  transport->invokeIdleTimeout();
+}
+
 TEST_F(QuicTransportImplTest, WriteAckPacketUnsetsLooper) {
   // start looper in running state first
   transport->writeLooper()->run(true);
diff --git a/quic/state/QuicStreamManager.h b/quic/state/QuicStreamManager.h
index dd4d65bb7..5d904a5b2 100644
--- a/quic/state/QuicStreamManager.h
+++ b/quic/state/QuicStreamManager.h
@@ -725,6 +725,13 @@ class QuicStreamManager {
     return streams_.size() != numControlStreams_;
   }
 
+  /*
+   * Returns number of control streams.
+   */
+  auto numControlStreams() {
+    return numControlStreams_;
+  }
+
   /*
    * Sets the given stream to be tracked as a control stream.
    */