Skip to content

Commit 1bcb3e1

Browse files
elrrrrrrrclaude
andcommitted
fix(pm): preserve clone error chain instead of swallowing it
Previously clone_package_once() returned bool and used .ok()? to discard the error from clone_package(). The caller in install.rs could only bail with a generic "{name} clone failed" message, losing the underlying IO error (e.g. NTFS hardlink failure on Windows). Now clone_package_once() returns Result<()> and the full error chain propagates to the user. The warn log also uses {:#} to print the complete error chain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f36dbd8 commit 1bcb3e1

3 files changed

Lines changed: 38 additions & 24 deletions

File tree

crates/pm/src/service/install.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,15 @@ pub async fn install_packages(
136136
package.optional == Some(true) || package.dev_optional == Some(true);
137137

138138
let task = tokio::spawn(async move {
139-
if !clone_package_once(&name, &version, &resolved, &target_path).await {
139+
if let Err(e) =
140+
clone_package_once(&name, &version, &resolved, &target_path).await
141+
{
140142
if is_optional {
141143
tracing::warn!("Optional dependency {name} failed (ignored)");
142144
PROGRESS_BAR.inc(1);
143145
return Ok(());
144146
}
145-
anyhow::bail!("{name} clone failed");
147+
return Err(e);
146148
}
147149
PROGRESS_BAR.inc(1);
148150
log_progress(&format!("{name} resolved"));

crates/pm/src/service/pipeline/worker.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub fn start_workers(channels: PipelineChannels, cwd: PathBuf) -> PipelineHandle
5959
if let Some(ref parent) = parent_path {
6060
wait_clone_if_pending(&parent.to_string_lossy()).await;
6161
}
62-
clone_package_once(&name, &version, &tarball_url, &target).await;
62+
let _ = clone_package_once(&name, &version, &tarball_url, &target).await;
6363
});
6464
}
6565
});

crates/pm/src/util/cloner.rs

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub async fn clone_package_once(
4141
version: &str,
4242
tarball_url: &str,
4343
target_path: &Path,
44-
) -> bool {
44+
) -> Result<()> {
4545
let key = target_path.to_string_lossy().to_string();
4646
let name = name.to_string();
4747
let version = version.to_string();
@@ -52,28 +52,40 @@ pub async fn clone_package_once(
5252
// so skip `find_real_src` which would incorrectly pick a subdirectory.
5353
let is_git = is_git_url(&tarball_url);
5454

55-
CLONE_CACHE
56-
.get_or_init(key, || async move {
57-
let cache_path = resolve_cache_path(&name, &version, &tarball_url).await?;
58-
clone_package(&cache_path, &target_path, &name, &version, !is_git)
59-
.await
60-
.inspect_err(|e| {
61-
tracing::warn!(
62-
"Clone failed: {}@{} to {}: {}",
63-
name,
64-
version,
65-
target_path.display(),
66-
e
67-
)
68-
})
69-
.ok()?;
70-
71-
CLONE_COUNT.fetch_add(1, Ordering::Relaxed);
72-
tracing::debug!("Cloned: {}@{} to {}", name, version, target_path.display());
73-
Some(())
55+
let target_display = target_path.display().to_string();
56+
57+
let ok = CLONE_CACHE
58+
.get_or_init(key, || {
59+
let name = name.clone();
60+
let version = version.clone();
61+
async move {
62+
let cache_path = resolve_cache_path(&name, &version, &tarball_url).await?;
63+
clone_package(&cache_path, &target_path, &name, &version, !is_git)
64+
.await
65+
.inspect_err(|e| {
66+
tracing::warn!(
67+
"Clone failed: {}@{} to {}: {:#}",
68+
name,
69+
version,
70+
target_path.display(),
71+
e
72+
)
73+
})
74+
.ok()?;
75+
76+
CLONE_COUNT.fetch_add(1, Ordering::Relaxed);
77+
tracing::debug!("Cloned: {}@{} to {}", name, version, target_path.display());
78+
Some(())
79+
}
7480
})
7581
.await
76-
.is_some()
82+
.is_some();
83+
84+
if ok {
85+
Ok(())
86+
} else {
87+
anyhow::bail!("Failed to clone {name}@{version} to {target_display}")
88+
}
7789
}
7890

7991
#[cfg(target_os = "macos")]

0 commit comments

Comments
 (0)