fix(dgw): downgrade benign client disconnects from ERROR to DEBUG (#1620)

Reduces log noise by treating common socket disconnections (BrokenPipe,
ConnectionReset, UnexpectedEof) as benign events during HTTP/HTTPS
serving and TLS handshake. These disconnects typically occur from health
checks, port scanners, aborted browser requests, or early connection
termination, and do not indicate server faults.

ERROR logs now only appear for genuine server issues, making it easier
to identify actionable problems in production deployments.

Issue: DGW-319

Author: CBenoit

devolutions-gateway/src/listener.rs

@@ -4,7 +4,6 @@ use anyhow::Context;
 use async_trait::async_trait;
 use devolutions_gateway_task::{ChildTask, ShutdownSignal, Task};
 use futures::TryFutureExt as _;
-use tap::Pipe as _;
 use tokio::io::{AsyncRead, AsyncWrite};
 use tokio::net::{TcpListener, TcpSocket, TcpStream};
 use tracing::Instrument as _;
@@ -220,17 +219,39 @@ async fn run_https_listener(listener: TcpListener, state: DgwState) -> anyhow::R
     }
 }
 
+/// Checks if an error represents a benign client disconnect.
+///
+/// Walks the error chain and returns true if any cause is a `std::io::Error`
+/// with kind `BrokenPipe`, `ConnectionReset`, or `UnexpectedEof`.
+fn is_benign_disconnect(err: &anyhow::Error) -> bool {
+    use std::io::ErrorKind::{BrokenPipe, ConnectionReset, UnexpectedEof};
+
+    err.chain().any(|cause| {
+        if let Some(ioe) = cause.downcast_ref::<std::io::Error>() {
+            return matches!(ioe.kind(), BrokenPipe | ConnectionReset | UnexpectedEof);
+        }
+        false
+    })
+}
+
 async fn handle_https_peer(
     stream: TcpStream,
     tls_acceptor: tokio_rustls::TlsAcceptor,
     state: DgwState,
     peer_addr: SocketAddr,
 ) -> anyhow::Result<()> {
-    let tls_stream = tls_acceptor
-        .accept(stream)
-        .await
-        .context("TLS handshake failed")?
-        .pipe(tokio_rustls::TlsStream::Server);
+    let tls_stream = match tls_acceptor.accept(stream).await {
+        Ok(stream) => tokio_rustls::TlsStream::Server(stream),
+        Err(e) => {
+            let e = anyhow::Error::from(e);
+            if is_benign_disconnect(&e) {
+                debug!(error = format!("{e:#}"), %peer_addr, "TLS handshake ended by peer");
+                return Ok(());
+            } else {
+                return Err(e.context("TLS handshake failed"));
+            }
+        }
+    };
 
     handle_http_peer(tls_stream, state, peer_addr).await
 }
@@ -259,13 +280,24 @@ where
 
     match result {
         Ok(()) => Ok(()),
-        Err(e) => match e.downcast_ref::<hyper::Error>() {
-            Some(e) if e.is_canceled() || e.is_incomplete_message() => {
-                debug!(error = %e, "Request was cancelled");
+        Err(error) => {
+            // Check for hyper-specific benign cases first.
+            if let Some(hyper_err) = error.downcast_ref::<hyper::Error>()
+                && (hyper_err.is_canceled() || hyper_err.is_incomplete_message())
+            {
+                debug!(error = format!("{:#}", anyhow::anyhow!(error)), %peer_addr, "Request was cancelled/incomplete");
+                return Ok(());
+            }
+
+            // Then check for underlying io::Error kinds via anyhow chain.
+            let error = anyhow::Error::from_boxed(error);
+            if is_benign_disconnect(&error) {
+                debug!(error = format!("{error:#}"), %peer_addr, "Client disconnected");
                 Ok(())
+            } else {
+                Err(error.context("HTTP server"))
             }
-            _ => Err(anyhow::anyhow!(e).context("HTTP server")),
-        },
+        }
     }
 }
 

testsuite/tests/cli/dgw/benign_disconnect.rs

@@ -0,0 +1,112 @@
+//! Tests for benign client disconnect handling.
+//!
+//! These tests verify that common client disconnects (e.g., health checks, aborted
+//! connections) are logged at DEBUG level instead of ERROR.
+//!
+//! Note: HTTPS disconnect handling is implemented in handle_https_peer and uses the same
+//! benign disconnect detection logic. However, testing HTTPS requires TLS configuration
+//! which is not included in the default test setup.
+
+use anyhow::Context as _;
+use rstest::rstest;
+use testsuite::cli::{dgw_tokio_cmd, wait_for_tcp_port};
+use testsuite::dgw_config::{DgwConfig, DgwConfigHandle, VerbosityProfile};
+use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
+use tokio::process::Child;
+
+/// Starts a gateway instance and returns the process and a handle to collect stderr.
+///
+/// The gateway is configured with DEBUG logging enabled to capture disconnect logs.
+/// Stderr is collected in a background task and returned when the handle is awaited.
+async fn start_gateway_with_logs(
+    config_handle: &DgwConfigHandle,
+) -> anyhow::Result<(Child, tokio::task::JoinHandle<Vec<String>>)> {
+    let mut process = dgw_tokio_cmd()
+        .env("DGATEWAY_CONFIG_PATH", config_handle.config_dir())
+        .env("RUST_LOG", "devolutions_gateway=debug")
+        .kill_on_drop(true)
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
+        .spawn()
+        .context("failed to start Devolutions Gateway")?;
+
+    let stderr = process.stderr.take().context("failed to take stderr")?;
+    let stderr_handle = tokio::spawn(async move {
+        let mut stderr_reader = BufReader::new(stderr);
+        let mut lines = Vec::new();
+        let mut line_buf = String::new();
+        loop {
+            match stderr_reader.read_line(&mut line_buf).await {
+                Ok(0) => break,
+                Ok(_) => {
+                    lines.push(line_buf.clone());
+                    line_buf.clear();
+                }
+                Err(_) => break,
+            }
+        }
+        lines
+    });
+
+    // Wait for HTTP port to be ready.
+    wait_for_tcp_port(config_handle.http_port()).await?;
+
+    Ok((process, stderr_handle))
+}
+
+/// Test that benign HTTP disconnects log DEBUG, not ERROR.
+///
+/// Tests various scenarios where clients disconnect without error:
+/// - Connecting and immediately closing (e.g., health checks, port scanners)
+/// - Sending partial request then closing (e.g., aborted browser requests)
+#[rstest]
+#[case::connect_and_close(None)]
+#[case::abort_mid_request(Some("GET /jet/health HTTP/1.1\r\nHost: localhost\r\n".as_bytes()))]
+#[tokio::test]
+async fn benign_http_disconnect(#[case] payload: Option<&[u8]>) -> anyhow::Result<()> {
+    // 1) Start the gateway with DEBUG logging.
+    let config_handle = DgwConfig::builder()
+        .disable_token_validation(true)
+        .verbosity_profile(VerbosityProfile::DEBUG)
+        .build()
+        .init()
+        .context("init config")?;
+
+    let (mut process, stderr_handle) = start_gateway_with_logs(&config_handle).await?;
+
+    // 2) Connect to HTTP port.
+    let mut stream = tokio::net::TcpStream::connect(("127.0.0.1", config_handle.http_port()))
+        .await
+        .context("failed to connect to HTTP port")?;
+
+    // 3) Send payload if provided.
+    if let Some(data) = payload {
+        stream.write_all(data).await.context("failed to send payload")?;
+    }
+
+    // 4) Close the connection.
+    drop(stream);
+
+    // Wait a bit for the log to be written.
+    tokio::time::sleep(tokio::time::Duration::from_millis(500)).await;
+
+    // 5) Stop the gateway and collect logs.
+    let _ = process.start_kill();
+    let stderr_lines = tokio::time::timeout(tokio::time::Duration::from_secs(5), stderr_handle)
+        .await
+        .context("timeout waiting for stderr")?
+        .context("wait for stderr collection")?;
+    let _ = process.wait().await;
+
+    // 6) Verify no ERROR logs about "HTTP server" or "handle_http_peer failed".
+    let has_error = stderr_lines.iter().any(|line| {
+        line.contains("ERROR") && (line.contains("HTTP server") || line.contains("handle_http_peer failed"))
+    });
+
+    assert!(
+        !has_error,
+        "Expected no ERROR logs for benign HTTP disconnect, but found one"
+    );
+
+    Ok(())
+}

testsuite/tests/cli/dgw/mod.rs

@@ -1,4 +1,5 @@
 mod ai_gateway;
+mod benign_disconnect;
 mod preflight;
 mod tls_anchoring;
 mod traffic_audit;