diff --git a/src/frame/go_away.rs b/src/frame/go_away.rs index 4ab28d514..99330e981 100644 --- a/src/frame/go_away.rs +++ b/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) -> 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, } } diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 7ea124e44..727643a65 100644 --- a/src/proto/connection.rs +++ b/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 Drop for Connection diff --git a/src/proto/error.rs b/src/proto/error.rs index 2c00c7ea6..ad023317e 100644 --- a/src/proto/error.rs +++ 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) -> 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) } diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 0fe2bdd57..cfc357082 100644 --- a/src/proto/streams/recv.rs +++ b/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", + )); } } diff --git a/src/server.rs b/src/server.rs index 032c0d17a..f1f4cf470 100644 --- a/src/server.rs +++ b/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 diff --git a/tests/h2-support/src/frames.rs b/tests/h2-support/src/frames.rs index 4ee20dd77..d302d3ce5 100644 --- a/tests/h2-support/src/frames.rs +++ b/tests/h2-support/src/frames.rs @@ -309,11 +309,19 @@ impl Mock { where I: Into, { - 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(), + )) } } diff --git a/tests/h2-tests/tests/server.rs b/tests/h2-tests/tests/server.rs index 78f4891a1..c8c1c9d1c 100644 --- a/tests/h2-tests/tests/server.rs +++ b/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!(); diff --git a/tests/h2-tests/tests/stream_states.rs b/tests/h2-tests/tests/stream_states.rs index 138328efa..c28066d2c 100644 --- a/tests/h2-tests/tests/stream_states.rs +++ b/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");