fix(logging): distinguish cancelled Git fetches from process failures

Production upload-pack errors had empty stderr and no repository or exit status, making normal client cancellation indistinguishable from a server failure. Retain the repository span across the detached streaming task and include process status and pump outcome for unexpected exits.

Client-disconnected pumps still kill and reap the child and retain unsuccessful clone accounting, but now log at debug level. Git protocol responses and subprocess lifetime behavior are unchanged; this does not claim that the observed production errors were all cancellations.

Validation: 921 library tests, 57 Git clone target tests, and 3 streaming integration tests pass. Clippy passes for all targets with warnings denied; formatting and diff checks pass.

Assisted-by: GPT-6
This commit is contained in:
DanConwayDev
2026-09-23 12:24:12 +00:00
parent 9c9e1176f2
commit 81aea4a69d
3 changed files with 27 additions and 7 deletions
+2
View File
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Include repository, process exit status, and stream outcome in Git fetch
failure diagnostics; log client-cancelled upload-pack streams at debug level.
- Pace descendant fallback cycles with a one-minute refresh delay while
retaining overlap and immediate baselines for changed frontiers. Move routine
batch-completion bookkeeping to debug logs.
+5
View File
@@ -204,6 +204,11 @@ pub async fn handle_receive_pack(
See [`src/git/handlers.rs:22-98`](src/git/handlers.rs:22-98) for the info-refs implementation.
The upload-pack streaming task retains a repository tracing span. Unexpected
process failures include the exit status and stream outcome even when Git
produces no stderr. A disconnected HTTP client cancels the child and is logged
at debug level; the operation remains unsuccessful in clone metrics.
#### [`authorization.rs`](src/git/authorization.rs) - Push Validation
**Core Logic:**
+20 -7
View File
@@ -14,7 +14,7 @@ use std::time::Duration;
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio::sync::mpsc;
use tokio::time::MissedTickBehavior;
use tracing::{debug, error, info, warn};
use tracing::{debug, error, info, warn, Instrument};
use super::protocol::{GitService, PktLine};
use super::storage::{FamilyKey, FamilyWriteLease, LocalGitStorage};
@@ -509,9 +509,13 @@ pub async fn handle_upload_pack(
// until EOF so the request future can return the HTTP body immediately.
let (tx, rx) = mpsc::channel::<Result<Frame<Bytes>, io::Error>>(STREAM_CHANNEL_DEPTH);
tokio::spawn(async move {
stream_upload_pack_output(git, stdout, stderr, tx, repo_lifecycle_guard, metrics).await;
});
let span = tracing::info_span!("git_upload_pack", repository = %repo_path.display());
tokio::spawn(
async move {
stream_upload_pack_output(git, stdout, stderr, tx, repo_lifecycle_guard, metrics).await;
}
.instrument(span),
);
Ok(streaming_response(GitService::UploadPack, rx))
}
@@ -554,6 +558,12 @@ async fn stream_upload_pack_output<S, E>(
None => Vec::new(),
};
if matches!(pump_result, PumpResult::ClientDisconnected) {
record_git_operation(&metrics, "clone", "error");
debug!(exit_status = %status, "Git upload-pack cancelled after client disconnected");
return;
}
if !status.success() && matches!(pump_result, PumpResult::Eof { sent_stdout: false }) {
record_git_operation(&metrics, "clone", "error");
let stderr_str = String::from_utf8_lossy(&stderr_output);
@@ -575,7 +585,8 @@ async fn stream_upload_pack_output<S, E>(
stderr_str
);
} else {
error!("Git upload-pack failed: {}", stderr_str);
error!(exit_status = %status, stream_outcome = ?pump_result,
stderr = %stderr_str, "Git upload-pack failed");
}
// The streaming response headers have already been sent. If Git failed
@@ -600,8 +611,10 @@ async fn stream_upload_pack_output<S, E>(
);
} else {
error!(
"Git upload-pack failed after streaming stdout: {}",
stderr_str
exit_status = %status,
stream_outcome = ?pump_result,
stderr = %stderr_str,
"Git upload-pack failed after streaming stdout"
);
}
} else if matches!(pump_result, PumpResult::Eof { .. }) {