Skip to content

Commit f146c90

Browse files
authored
feat: improve download failure reporting (#64)
* feat: improve download failure messaging * test: cover download failure reporting * fix: refine download failure feedback * test: cover clipboard status feedback * feat: improve download failure reporting Remove unused IPC counters while keeping structured failure details deterministic and safe. * test: cover download failure reporting
1 parent c294fb9 commit f146c90

9 files changed

Lines changed: 1446 additions & 193 deletions

File tree

src-tauri/src/downloader.rs

Lines changed: 499 additions & 67 deletions
Large diffs are not rendered by default.

src-tauri/src/lib.rs

Lines changed: 88 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ mod downloader;
33
mod parser;
44

55
use converter::get_converter;
6-
use downloader::{DownloadResult, Downloader, ProgressEvent};
6+
use downloader::{DownloadResult, Downloader, FailureSummary, ProgressEvent};
77
use parser::{normalize_split, parse_ndjson, ImageEntry, NDJSONData, ParseError};
88
use serde::Serialize;
99
use std::collections::{HashMap, HashSet};
@@ -19,12 +19,42 @@ const MAX_DOWNLOAD_CONCURRENCY: usize = 20;
1919
#[derive(Debug, Serialize)]
2020
pub struct ConvertResult {
2121
pub zip_path: String,
22-
pub file_count: usize,
23-
pub image_count: usize,
24-
pub download_total: u32,
25-
pub failed_downloads: usize,
2622
pub omitted_images: usize,
27-
pub expired_url_failures: usize,
23+
pub failure_summary: FailureSummary,
24+
}
25+
26+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
27+
#[serde(rename_all = "snake_case")]
28+
pub enum ConvertErrorKind {
29+
ConversionFailed,
30+
DownloadFailed,
31+
}
32+
33+
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
34+
pub struct ConvertError {
35+
pub kind: ConvertErrorKind,
36+
pub message: String,
37+
pub failure_summary: Option<FailureSummary>,
38+
}
39+
40+
impl ConvertError {
41+
fn download_failed(failure_summary: &FailureSummary) -> Self {
42+
Self {
43+
kind: ConvertErrorKind::DownloadFailed,
44+
message: "Images could not be downloaded.".to_string(),
45+
failure_summary: Some(failure_summary.clone()),
46+
}
47+
}
48+
}
49+
50+
impl From<String> for ConvertError {
51+
fn from(message: String) -> Self {
52+
Self {
53+
kind: ConvertErrorKind::ConversionFailed,
54+
message,
55+
failure_summary: None,
56+
}
57+
}
2858
}
2959

3060
fn normalize_zip_path(path: &str) -> Result<String, String> {
@@ -227,27 +257,13 @@ fn validate_downloaded_image_count(
227257
include_images: bool,
228258
original_image_count: usize,
229259
kept_image_count: usize,
230-
download_total: u32,
231-
expired_url_failures: usize,
232-
) -> Result<(), String> {
260+
failure_summary: &FailureSummary,
261+
) -> Result<(), ConvertError> {
233262
if !include_images || original_image_count == 0 || kept_image_count > 0 {
234263
return Ok(());
235264
}
236265

237-
if download_total == 0 {
238-
return Err(
239-
"No image URLs were found in this export. Re-export the dataset and try again."
240-
.to_string(),
241-
);
242-
}
243-
244-
let mut message =
245-
"All image downloads failed. Check your network or CDN access and try again.".to_string();
246-
if expired_url_failures > 0 {
247-
message
248-
.push_str(" Some signed URLs may have expired; re-export the dataset and try again.");
249-
}
250-
Err(message)
266+
Err(ConvertError::download_failed(failure_summary))
251267
}
252268

253269
#[tauri::command]
@@ -257,15 +273,16 @@ async fn convert_ndjson(
257273
output_path: String,
258274
include_images: bool,
259275
channel: Channel<ProgressEvent>,
260-
) -> Result<ConvertResult, String> {
276+
) -> Result<ConvertResult, ConvertError> {
261277
let metadata = std::fs::metadata(&file_path)
262278
.map_err(|e| format!("Failed to inspect file '{}': {}", file_path, e))?;
263279
if !is_ndjson_size_allowed(metadata.len()) {
264280
return Err(format!(
265281
"NDJSON file is too large ({} bytes). Maximum allowed is {} bytes.",
266282
metadata.len(),
267283
MAX_NDJSON_BYTES
268-
));
284+
)
285+
.into());
269286
}
270287

271288
// Read the NDJSON file
@@ -303,7 +320,8 @@ async fn convert_ndjson(
303320
"The '{}' format does not support semantic segmentation datasets. \
304321
Use YOLO, COCO, or Pascal VOC.",
305322
format
306-
));
323+
)
324+
.into());
307325
}
308326

309327
// Semantic datasets carry polygon segments. PNG-mask-origin exports arrive
@@ -312,7 +330,8 @@ async fn convert_ndjson(
312330
if semantic_dataset_has_no_polygons(&data) {
313331
return Err("Semantic dataset has no polygon segments in any image. \
314332
PNG-mask exports are not supported; provide polygon annotations."
315-
.to_string());
333+
.to_string()
334+
.into());
316335
}
317336

318337
// Download images if requested
@@ -323,26 +342,23 @@ async fn convert_ndjson(
323342
} else {
324343
DownloadResult {
325344
files: std::collections::HashMap::new(),
326-
total: 0,
327-
failed: 0,
328-
expired_url_failures: 0,
345+
failure_summary: Default::default(),
329346
}
330347
};
331348

332-
let download_total = download_result.total;
333-
let failed_downloads = download_result.failed;
334-
let expired_url_failures = download_result.expired_url_failures;
335349
let omitted_images =
336350
filter_images_without_downloads(&mut data, &download_result.files, include_images);
337351
let kept_image_count = data.images.len();
338352
validate_downloaded_image_count(
339353
include_images,
340354
original_image_count,
341355
kept_image_count,
342-
download_total,
343-
expired_url_failures,
356+
&download_result.failure_summary,
344357
)?;
345-
let image_count = download_result.files.len();
358+
let DownloadResult {
359+
files: downloaded_images,
360+
failure_summary,
361+
} = download_result;
346362

347363
// Get converter
348364
let converter = get_converter(&format).ok_or_else(|| format!("Unknown format: {}", format))?;
@@ -357,7 +373,7 @@ async fn convert_ndjson(
357373
})
358374
.ok();
359375

360-
let files = converter.convert(&data, &download_result.files);
376+
let files = converter.convert(&data, &downloaded_images);
361377

362378
channel
363379
.send(ProgressEvent {
@@ -419,7 +435,7 @@ async fn convert_ndjson(
419435

420436
if let Err(err) = zip_result {
421437
let _ = std::fs::remove_file(&output_path);
422-
return Err(err);
438+
return Err(err.into());
423439
}
424440

425441
channel
@@ -433,12 +449,8 @@ async fn convert_ndjson(
433449

434450
Ok(ConvertResult {
435451
zip_path: output_path.to_string_lossy().to_string(),
436-
file_count: files.len(),
437-
image_count,
438-
download_total,
439-
failed_downloads,
440452
omitted_images,
441-
expired_url_failures,
453+
failure_summary,
442454
})
443455
}
444456

@@ -460,9 +472,10 @@ mod tests {
460472
file_name_with_suffix, filter_images_without_downloads, is_ndjson_size_allowed,
461473
normalize_zip_path, prepare_images_with_unique_output_names,
462474
semantic_dataset_has_no_polygons, semantic_format_supported, short_stable_hash,
463-
validate_downloaded_image_count, validate_pose_dataset, MAX_NDJSON_BYTES,
475+
validate_downloaded_image_count, validate_pose_dataset, ConvertErrorKind, MAX_NDJSON_BYTES,
464476
};
465477
use crate::converter::get_converter;
478+
use crate::downloader::{FailureGroup, FailureKind, FailureSummary};
466479
use crate::parser::{image_entry_download_key, parse_ndjson};
467480
use std::collections::HashMap;
468481

@@ -530,37 +543,49 @@ mod tests {
530543

531544
#[test]
532545
fn all_missing_images_fail_even_when_no_download_was_attempted() {
533-
let error = validate_downloaded_image_count(true, 2, 0, 0, 0).unwrap_err();
546+
let summary = failure_summary(FailureKind::MissingUrl, 2);
547+
let error = validate_downloaded_image_count(true, 2, 0, &summary).unwrap_err();
534548

535-
assert_eq!(
536-
error,
537-
"No image URLs were found in this export. Re-export the dataset and try again."
538-
);
549+
assert_eq!(error.kind, ConvertErrorKind::DownloadFailed);
550+
assert_eq!(error.failure_summary, Some(summary));
551+
assert_eq!(error.message, "Images could not be downloaded.");
539552
}
540553

541554
#[test]
542-
fn all_network_failures_return_generic_retry_message() {
543-
let error = validate_downloaded_image_count(true, 2, 0, 2, 0).unwrap_err();
555+
fn all_network_failures_return_structured_summary() {
556+
let summary = failure_summary(FailureKind::Connect, 2);
557+
let error = validate_downloaded_image_count(true, 2, 0, &summary).unwrap_err();
544558

545-
assert_eq!(
546-
error,
547-
"All image downloads failed. Check your network or CDN access and try again."
548-
);
549-
assert!(!error.contains("signed URLs"));
559+
assert_eq!(error.kind, ConvertErrorKind::DownloadFailed);
560+
assert_eq!(error.failure_summary, Some(summary));
550561
}
551562

552563
#[test]
553-
fn all_expired_urls_include_reexport_hint() {
554-
let error = validate_downloaded_image_count(true, 2, 0, 2, 2).unwrap_err();
555-
556-
assert!(error.contains("signed URLs may have expired"));
557-
assert!(error.contains("re-export"));
564+
fn all_expired_urls_return_structured_summary() {
565+
let summary = failure_summary(FailureKind::ExpiredUrl, 2);
566+
let error = validate_downloaded_image_count(true, 2, 0, &summary).unwrap_err();
567+
568+
assert_eq!(error.kind, ConvertErrorKind::DownloadFailed);
569+
assert_eq!(error.failure_summary, Some(summary));
570+
}
571+
572+
fn failure_summary(kind: FailureKind, count: usize) -> FailureSummary {
573+
FailureSummary {
574+
groups: vec![FailureGroup {
575+
kind,
576+
count,
577+
examples: vec!["example.jpg".to_string()],
578+
http_statuses: Vec::new(),
579+
}],
580+
expiry: None,
581+
}
558582
}
559583

560584
#[test]
561585
fn zero_image_and_labels_only_datasets_do_not_fail_download_validation() {
562-
assert!(validate_downloaded_image_count(true, 0, 0, 0, 0).is_ok());
563-
assert!(validate_downloaded_image_count(false, 2, 0, 0, 0).is_ok());
586+
let summary = FailureSummary::default();
587+
assert!(validate_downloaded_image_count(true, 0, 0, &summary).is_ok());
588+
assert!(validate_downloaded_image_count(false, 2, 0, &summary).is_ok());
564589
}
565590

566591
#[test]

0 commit comments

Comments
 (0)