From 22d93ea5e707e99d67ab367d60c7a9d867b7665c Mon Sep 17 00:00:00 2001
From: DanConwayDev <DanConwayDev@protonmail.com>
Date: Tue, 3 Feb 2026 22:00:31 +0000
Subject: Add error logging to all git handler IO operations

Previously, some IO errors in git handlers were logged while others were
not, leading to inconsistent observability. Additionally, the HTTP layer
logged all git errors redundantly, adding no useful context beyond what
was already logged at the source.

Changes:
- Add error logging to all previously unlogged IO operations in
  handle_upload_pack and handle_receive_pack (stdin writes, stdout/stderr
  reads, process waits)
- Remove redundant error logging at HTTP layer since all errors are now
  logged at their source with full context
- Ensures consistent error-level logging for all git subprocess failures

This provides complete observability of git operations while eliminating
duplicate log entries that don't add value.
---
 src/git/handlers.rs | 40 ++++++++++++++++++++++++++++++++--------
 src/http/mod.rs     |  2 +-
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/git/handlers.rs b/src/git/handlers.rs
index 7244abb..28cb47f 100644
--- a/src/git/handlers.rs
+++ b/src/git/handlers.rs
@@ -156,7 +156,10 @@ pub async fn handle_upload_pack(
         stdin
             .write_all(&request_body)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to write to git upload-pack stdin: {}", e);
+                GitError::IoError(e)
+            })?;
         // Close stdin to signal end of input
         drop(stdin);
     }
@@ -170,7 +173,10 @@ pub async fn handle_upload_pack(
         stdout
             .read_to_end(&mut output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git upload-pack stdout: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     if let Some(stderr) = git.take_stderr() {
@@ -178,11 +184,17 @@ pub async fn handle_upload_pack(
         stderr
             .read_to_end(&mut stderr_output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git upload-pack stderr: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     // Wait for process
-    let status = git.wait().await.map_err(GitError::IoError)?;
+    let status = git.wait().await.map_err(|e| {
+        error!("Failed to wait for git upload-pack process: {}", e);
+        GitError::IoError(e)
+    })?;
 
     if !status.success() {
         let stderr_str = String::from_utf8_lossy(&stderr_output);
@@ -299,7 +311,10 @@ pub async fn handle_receive_pack(
         stdin
             .write_all(&request_body)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to write to git receive-pack stdin: {}", e);
+                GitError::IoError(e)
+            })?;
         drop(stdin);
     }
 
@@ -312,7 +327,10 @@ pub async fn handle_receive_pack(
         stdout
             .read_to_end(&mut output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git receive-pack stdout: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     if let Some(stderr) = git.take_stderr() {
@@ -320,11 +338,17 @@ pub async fn handle_receive_pack(
         stderr
             .read_to_end(&mut stderr_output)
             .await
-            .map_err(GitError::IoError)?;
+            .map_err(|e| {
+                error!("Failed to read git receive-pack stderr: {}", e);
+                GitError::IoError(e)
+            })?;
     }
 
     // Wait for process
-    let status = git.wait().await.map_err(GitError::IoError)?;
+    let status = git.wait().await.map_err(|e| {
+        error!("Failed to wait for git receive-pack process: {}", e);
+        GitError::IoError(e)
+    })?;
 
     if !status.success() {
         let stderr_str = String::from_utf8_lossy(&stderr_output);
diff --git a/src/http/mod.rs b/src/http/mod.rs
index ffb1562..edc28a3 100644
--- a/src/http/mod.rs
+++ b/src/http/mod.rs
@@ -332,7 +332,7 @@ impl Service<Request<Incoming>> for HttpService {
                             .unwrap())
                     }
                     Err(e) => {
-                        tracing::error!("Git handler error: {}", e);
+                        // Errors are already logged at their source with full context
                         let error_msg = format!("Git error: {}", e);
                         Ok(add_cors_headers(Response::builder())
                             .status(e.status_code())
-- 
cgit v1.2.3