Skip to content

Commit c1a2bc0

Browse files
Wenxin-Jiangclaude
andauthored
fix: add error handling to get command for blob writes, decodes, and dir creation (#54)
Replace silent .ok() calls on directory creation, blob writes, and base64 decodes with proper error propagation. Fix the always-exit-0 bug by returning non-zero exit codes on partial failures and apply failures. Track apply outcome to accurately report "applied" count in JSON output. Apply the same fixes to both batch and one-off (save_and_apply_patch) code paths. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1d59887 commit c1a2bc0

File tree

1 file changed

+149
-24
lines changed
  • crates/socket-patch-cli/src/commands

1 file changed

+149
-24
lines changed

crates/socket-patch-cli/src/commands/get.rs

Lines changed: 149 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,30 @@ pub async fn download_and_apply_patches(
267267
let blobs_dir = socket_dir.join("blobs");
268268
let manifest_path = socket_dir.join("manifest.json");
269269

270-
tokio::fs::create_dir_all(&socket_dir).await.ok();
271-
tokio::fs::create_dir_all(&blobs_dir).await.ok();
270+
if let Err(e) = tokio::fs::create_dir_all(&socket_dir).await {
271+
let err = format!("Failed to create .socket directory: {}", e);
272+
if params.json {
273+
println!("{}", serde_json::to_string_pretty(&serde_json::json!({
274+
"status": "error",
275+
"error": &err,
276+
})).unwrap());
277+
} else {
278+
eprintln!("Error: {}", &err);
279+
}
280+
return (1, serde_json::json!({"status": "error", "error": err}));
281+
}
282+
if let Err(e) = tokio::fs::create_dir_all(&blobs_dir).await {
283+
let err = format!("Failed to create blobs directory: {}", e);
284+
if params.json {
285+
println!("{}", serde_json::to_string_pretty(&serde_json::json!({
286+
"status": "error",
287+
"error": &err,
288+
})).unwrap());
289+
} else {
290+
eprintln!("Error: {}", &err);
291+
}
292+
return (1, serde_json::json!({"status": "error", "error": err}));
293+
}
272294

273295
let mut manifest = match read_manifest(&manifest_path).await {
274296
Ok(Some(m)) => m,
@@ -324,6 +346,7 @@ pub async fn download_and_apply_patches(
324346
}
325347

326348
// Save blob contents
349+
let mut patch_failed = false;
327350
let mut files = HashMap::new();
328351
for (file_path, file_info) in &patch.files {
329352
if let (Some(ref before), Some(ref after)) =
@@ -341,24 +364,63 @@ pub async fn download_and_apply_patches(
341364
if let (Some(ref blob_content), Some(ref after_hash)) =
342365
(&file_info.blob_content, &file_info.after_hash)
343366
{
344-
if let Ok(decoded) = base64_decode(blob_content) {
345-
let blob_path = blobs_dir.join(after_hash);
346-
tokio::fs::write(&blob_path, &decoded).await.ok();
367+
match base64_decode(blob_content) {
368+
Ok(decoded) => {
369+
let blob_path = blobs_dir.join(after_hash);
370+
if let Err(e) = tokio::fs::write(&blob_path, &decoded).await {
371+
if !params.json && !params.silent {
372+
eprintln!(" [error] Failed to write blob for {}: {}", file_path, e);
373+
}
374+
patch_failed = true;
375+
break;
376+
}
377+
}
378+
Err(e) => {
379+
if !params.json && !params.silent {
380+
eprintln!(" [error] Failed to decode blob for {}: {}", file_path, e);
381+
}
382+
patch_failed = true;
383+
break;
384+
}
347385
}
348386
}
349387

350388
// Also store beforeHash blob if present (needed for rollback)
351389
if let (Some(ref before_blob), Some(ref before_hash)) =
352390
(&file_info.before_blob_content, &file_info.before_hash)
353391
{
354-
if let Ok(decoded) = base64_decode(before_blob) {
355-
tokio::fs::write(blobs_dir.join(before_hash), &decoded)
356-
.await
357-
.ok();
392+
match base64_decode(before_blob) {
393+
Ok(decoded) => {
394+
if let Err(e) = tokio::fs::write(blobs_dir.join(before_hash), &decoded).await {
395+
if !params.json && !params.silent {
396+
eprintln!(" [error] Failed to write before-blob for {}: {}", file_path, e);
397+
}
398+
patch_failed = true;
399+
break;
400+
}
401+
}
402+
Err(e) => {
403+
if !params.json && !params.silent {
404+
eprintln!(" [error] Failed to decode before-blob for {}: {}", file_path, e);
405+
}
406+
patch_failed = true;
407+
break;
408+
}
358409
}
359410
}
360411
}
361412

413+
if patch_failed {
414+
patches_failed += 1;
415+
downloaded_patches.push(serde_json::json!({
416+
"purl": patch.purl,
417+
"uuid": patch.uuid,
418+
"action": "failed",
419+
"error": "Blob decode or write failed",
420+
}));
421+
continue;
422+
}
423+
362424
let vulnerabilities: HashMap<String, VulnerabilityInfo> = patch
363425
.vulnerabilities
364426
.iter()
@@ -454,6 +516,7 @@ pub async fn download_and_apply_patches(
454516
}
455517

456518
// Auto-apply unless --save-only
519+
let mut apply_succeeded = false;
457520
if !params.save_only && patches_added > 0 {
458521
if !params.json && !params.silent {
459522
eprintln!("\nApplying patches...");
@@ -472,23 +535,25 @@ pub async fn download_and_apply_patches(
472535
verbose: false,
473536
};
474537
let code = super::apply::run(apply_args).await;
538+
apply_succeeded = code == 0;
475539
if code != 0 && !params.json && !params.silent {
476540
eprintln!("\nSome patches could not be applied.");
477541
}
478542
}
479543

480544
let result_json = serde_json::json!({
481-
"status": "success",
545+
"status": if patches_failed > 0 { "partial_failure" } else { "success" },
482546
"found": selected.len(),
483547
"downloaded": patches_added,
484548
"skipped": patches_skipped,
485549
"failed": patches_failed,
486-
"applied": if !params.save_only && patches_added > 0 { patches_added } else { 0 },
550+
"applied": if apply_succeeded { patches_added } else { 0 },
487551
"updated": updates.len(),
488552
"patches": downloaded_patches,
489553
});
490554

491-
(0, result_json)
555+
let exit_code = if patches_failed > 0 || (!apply_succeeded && patches_added > 0 && !params.save_only) { 1 } else { 0 };
556+
(exit_code, result_json)
492557
}
493558

494559
pub async fn run(args: GetArgs) -> i32 {
@@ -962,14 +1027,25 @@ async fn save_and_apply_patch(
9621027
let blobs_dir = socket_dir.join("blobs");
9631028
let manifest_path = socket_dir.join("manifest.json");
9641029

965-
tokio::fs::create_dir_all(&blobs_dir).await.ok();
1030+
if let Err(e) = tokio::fs::create_dir_all(&blobs_dir).await {
1031+
if args.json {
1032+
println!("{}", serde_json::to_string_pretty(&serde_json::json!({
1033+
"status": "error",
1034+
"error": format!("Failed to create blobs directory: {}", e),
1035+
})).unwrap());
1036+
} else {
1037+
eprintln!("Error: Failed to create blobs directory: {}", e);
1038+
}
1039+
return 1;
1040+
}
9661041

9671042
let mut manifest = match read_manifest(&manifest_path).await {
9681043
Ok(Some(m)) => m,
9691044
_ => PatchManifest::new(),
9701045
};
9711046

9721047
// Build and save patch record
1048+
let mut blob_failed = false;
9731049
let mut files = HashMap::new();
9741050
for (file_path, file_info) in &patch.files {
9751051
if let Some(ref after) = file_info.after_hash {
@@ -987,24 +1063,71 @@ async fn save_and_apply_patch(
9871063
if let (Some(ref blob_content), Some(ref after_hash)) =
9881064
(&file_info.blob_content, &file_info.after_hash)
9891065
{
990-
if let Ok(decoded) = base64_decode(blob_content) {
991-
tokio::fs::write(blobs_dir.join(after_hash), &decoded)
992-
.await
993-
.ok();
1066+
match base64_decode(blob_content) {
1067+
Ok(decoded) => {
1068+
if let Err(e) = tokio::fs::write(blobs_dir.join(after_hash), &decoded).await {
1069+
if !args.json {
1070+
eprintln!(" [error] Failed to write blob for {}: {}", file_path, e);
1071+
}
1072+
blob_failed = true;
1073+
break;
1074+
}
1075+
}
1076+
Err(e) => {
1077+
if !args.json {
1078+
eprintln!(" [error] Failed to decode blob for {}: {}", file_path, e);
1079+
}
1080+
blob_failed = true;
1081+
break;
1082+
}
9941083
}
9951084
}
9961085
// Also store beforeHash blob if present (needed for rollback)
9971086
if let (Some(ref before_blob), Some(ref before_hash)) =
9981087
(&file_info.before_blob_content, &file_info.before_hash)
9991088
{
1000-
if let Ok(decoded) = base64_decode(before_blob) {
1001-
tokio::fs::write(blobs_dir.join(before_hash), &decoded)
1002-
.await
1003-
.ok();
1089+
match base64_decode(before_blob) {
1090+
Ok(decoded) => {
1091+
if let Err(e) = tokio::fs::write(blobs_dir.join(before_hash), &decoded).await {
1092+
if !args.json {
1093+
eprintln!(" [error] Failed to write before-blob for {}: {}", file_path, e);
1094+
}
1095+
blob_failed = true;
1096+
break;
1097+
}
1098+
}
1099+
Err(e) => {
1100+
if !args.json {
1101+
eprintln!(" [error] Failed to decode before-blob for {}: {}", file_path, e);
1102+
}
1103+
blob_failed = true;
1104+
break;
1105+
}
10041106
}
10051107
}
10061108
}
10071109

1110+
if blob_failed {
1111+
if args.json {
1112+
println!("{}", serde_json::to_string_pretty(&serde_json::json!({
1113+
"status": "error",
1114+
"found": 1,
1115+
"downloaded": 0,
1116+
"applied": 0,
1117+
"error": "Blob decode or write failed",
1118+
"patches": [{
1119+
"purl": patch.purl,
1120+
"uuid": patch.uuid,
1121+
"action": "failed",
1122+
"error": "Blob decode or write failed",
1123+
}],
1124+
})).unwrap());
1125+
} else {
1126+
eprintln!("Error: Blob decode or write failed for patch {}", patch.purl);
1127+
}
1128+
return 1;
1129+
}
1130+
10081131
let vulnerabilities: HashMap<String, VulnerabilityInfo> = patch
10091132
.vulnerabilities
10101133
.iter()
@@ -1060,7 +1183,8 @@ async fn save_and_apply_patch(
10601183
}
10611184
}
10621185

1063-
if !args.save_only {
1186+
let mut apply_succeeded = false;
1187+
if !args.save_only && added {
10641188
if !args.json {
10651189
println!("\nApplying patches...");
10661190
}
@@ -1078,6 +1202,7 @@ async fn save_and_apply_patch(
10781202
verbose: false,
10791203
};
10801204
let code = super::apply::run(apply_args).await;
1205+
apply_succeeded = code == 0;
10811206
if code != 0 && !args.json {
10821207
eprintln!("\nSome patches could not be applied.");
10831208
}
@@ -1088,7 +1213,7 @@ async fn save_and_apply_patch(
10881213
"status": "success",
10891214
"found": 1,
10901215
"downloaded": if added { 1 } else { 0 },
1091-
"applied": if !args.save_only && added { 1 } else { 0 },
1216+
"applied": if apply_succeeded { 1 } else { 0 },
10921217
"patches": [{
10931218
"purl": patch.purl,
10941219
"uuid": patch.uuid,
@@ -1097,7 +1222,7 @@ async fn save_and_apply_patch(
10971222
})).unwrap());
10981223
}
10991224

1100-
0
1225+
if !apply_succeeded && added && !args.save_only { 1 } else { 0 }
11011226
}
11021227

11031228
fn base64_decode(input: &str) -> Result<Vec<u8>, String> {

0 commit comments

Comments
 (0)