Add too_many_resets debug_data to the GOAWAY we send (#678)

Closes hyperium/hyper#3211

Author: Herbstein

src/frame/go_away.rs

@@ -20,12 +20,11 @@ impl GoAway {
         }
     }
 
-    #[doc(hidden)]
-    #[cfg(feature = "unstable")]
-    pub fn with_debug_data(self, debug_data: impl Into<Bytes>) -> Self {
+    pub fn with_debug_data(last_stream_id: StreamId, reason: Reason, debug_data: Bytes) -> Self {
         Self {
-            debug_data: debug_data.into(),
-            ..self
+            last_stream_id,
+            error_code: reason,
+            debug_data,
         }
     }
 

src/proto/connection.rs

@@ -398,16 +398,10 @@ where
         self.go_away.go_away_now(frame);
     }
 
-    #[doc(hidden)]
-    #[cfg(feature = "unstable")]
-    fn go_away_now_debug_data(&mut self) {
+    fn go_away_now_data(&mut self, e: Reason, data: Bytes) {
         let last_processed_id = self.streams.last_processed_id();
-
-        let frame = frame::GoAway::new(last_processed_id, Reason::NO_ERROR)
-            .with_debug_data("something went wrong");
-
-        self.streams.send_go_away(last_processed_id);
-        self.go_away.go_away(frame);
+        let frame = frame::GoAway::with_debug_data(last_processed_id, e, data);
+        self.go_away.go_away_now(frame);
     }
 
     fn go_away_from_user(&mut self, e: Reason) {
@@ -430,7 +424,7 @@ where
             // error. This is handled by setting a GOAWAY frame followed by
             // terminating the connection.
             Err(Error::GoAway(debug_data, reason, initiator)) => {
-                let e = Error::GoAway(debug_data, reason, initiator);
+                let e = Error::GoAway(debug_data.clone(), reason, initiator);
                 tracing::debug!(error = ?e, "Connection::poll; connection error");
 
                 // We may have already sent a GOAWAY for this error,
@@ -447,7 +441,7 @@ where
 
                 // Reset all active streams
                 self.streams.handle_error(e);
-                self.go_away_now(reason);
+                self.go_away_now_data(reason, debug_data);
                 Ok(())
             }
             // Attempting to read a frame resulted in a stream level error.
@@ -588,17 +582,6 @@ where
         // for a pong before proceeding.
         self.inner.ping_pong.ping_shutdown();
     }
-
-    #[doc(hidden)]
-    #[cfg(feature = "unstable")]
-    pub fn go_away_debug_data(&mut self) {
-        if self.inner.go_away.is_going_away() {
-            return;
-        }
-
-        self.inner.as_dyn().go_away_now_debug_data();
-        self.inner.ping_pong.ping_shutdown();
-    }
 }
 
 impl<T, P, B> Drop for Connection<T, P, B>

src/proto/error.rs

@@ -40,6 +40,10 @@ impl Error {
         Self::GoAway(Bytes::new(), reason, Initiator::Library)
     }
 
+    pub(crate) fn library_go_away_data(reason: Reason, debug_data: impl Into<Bytes>) -> Self {
+        Self::GoAway(debug_data.into(), reason, Initiator::Library)
+    }
+
     pub(crate) fn remote_reset(stream_id: StreamId, reason: Reason) -> Self {
         Self::Reset(stream_id, reason, Initiator::Remote)
     }

src/proto/streams/recv.rs

@@ -763,7 +763,10 @@ impl Recv {
                     "recv_reset; remotely-reset pending-accept streams reached limit ({:?})",
                     counts.max_remote_reset_streams(),
                 );
-                return Err(Error::library_go_away(Reason::ENHANCE_YOUR_CALM));
+                return Err(Error::library_go_away_data(
+                    Reason::ENHANCE_YOUR_CALM,
+                    "too_many_resets",
+                ));
             }
         }
 

src/server.rs

@@ -544,12 +544,6 @@ where
         self.connection.go_away_gracefully();
     }
 
-    #[doc(hidden)]
-    #[cfg(feature = "unstable")]
-    pub fn debug_data_shutdown(&mut self) {
-        self.connection.go_away_debug_data();
-    }
-
     /// Takes a `PingPong` instance from the connection.
     ///
     /// # Note

tests/h2-support/src/frames.rs

@@ -309,11 +309,19 @@ impl Mock<frame::GoAway> {
     where
         I: Into<Bytes>,
     {
-        Mock(self.0.with_debug_data(debug_data.into()))
+        Mock(frame::GoAway::with_debug_data(
+            self.0.last_stream_id(),
+            self.0.reason(),
+            debug_data.into(),
+        ))
     }
 
     pub fn reason(self, reason: frame::Reason) -> Self {
-        Mock(frame::GoAway::new(self.0.last_stream_id(), reason))
+        Mock(frame::GoAway::with_debug_data(
+            self.0.last_stream_id(),
+            reason,
+            self.0.debug_data().clone(),
+        ))
     }
 }
 

tests/h2-tests/tests/server.rs

@@ -705,41 +705,6 @@ async fn graceful_shutdown() {
     join(client, srv).await;
 }
 
-#[tokio::test]
-async fn go_away_sends_debug_data() {
-    h2_support::trace_init!();
-
-    let (io, mut client) = mock::new();
-
-    let client = async move {
-        let settings = client.assert_server_handshake().await;
-        assert_default_settings!(settings);
-        client
-            .send_frame(frames::headers(1).request("POST", "https://example.com/"))
-            .await;
-        client
-            .recv_frame(frames::go_away(1).no_error().data("something went wrong"))
-            .await;
-    };
-
-    let src = async move {
-        let mut srv = server::handshake(io).await.expect("handshake");
-        let (_req, _tx) = srv.next().await.unwrap().expect("server receives request");
-
-        srv.debug_data_shutdown();
-
-        let srv_fut = async move {
-            poll_fn(move |cx| srv.poll_closed(cx))
-                .await
-                .expect("server");
-        };
-
-        srv_fut.await
-    };
-
-    join(client, src).await;
-}
-
 #[tokio::test]
 async fn goaway_even_if_client_sent_goaway() {
     h2_support::trace_init!();

tests/h2-tests/tests/stream_states.rs

@@ -211,9 +211,14 @@ async fn reset_streams_dont_grow_memory_continuously() {
                 .await;
             client.send_frame(frames::reset(n).protocol_error()).await;
         }
+
         tokio::time::timeout(
             std::time::Duration::from_secs(1),
-            client.recv_frame(frames::go_away((MAX * 2 + 1) as u32).calm()),
+            client.recv_frame(
+                frames::go_away((MAX * 2 + 1) as u32)
+                    .data("too_many_resets")
+                    .calm(),
+            ),
         )
         .await
         .expect("client goaway");