mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(cli,fuse,gui): make --wait fail when an import or re-date did
`import-google-photos --wait` and `redate-photos --wait` exited 0 however the run ended, and a run that failed as a whole left no summary, so the CLI said no import had run and the app said it finished. Failed runs now leave a summary with the reason, `--wait` exits 1 on failure, cancel or failed photos, and the app shows the reason (B239).
This commit is contained in:
1 parent
937a0a6824
commit
4836c5a84a
15 files changed
+6259
-6102
No files matched your search
@@ -2598,7 +2598,10 @@ fn cmd_import_google_photos(archives: Vec<PathBuf>, dry_run: bool, wait: bool) -
|
||||
} => continue,
|
||||
CtlResponse::ImportStatus { summary, .. } => {
|
||||
print_import_summary(summary.as_ref());
|
||||
return Ok(());
|
||||
return finished_run(
|
||||
"the import",
|
||||
summary.map(|s| (s.failed, s.cancelled, s.error)),
|
||||
);
|
||||
}
|
||||
CtlResponse::Error { message, kind } => bail!("{}", cli_error(kind, &message)),
|
||||
other => bail!("unexpected response: {other:?}"),
|
||||
@@ -2641,7 +2644,10 @@ fn cmd_redate_photos(
|
||||
CtlResponse::RedateStatus { running: true, .. } => continue,
|
||||
CtlResponse::RedateStatus { summary, .. } => {
|
||||
print_redate_summary(summary.as_ref());
|
||||
return Ok(());
|
||||
return finished_run(
|
||||
"the re-date",
|
||||
summary.map(|s| (s.failed, s.cancelled, s.error)),
|
||||
);
|
||||
}
|
||||
CtlResponse::Error { message, kind } => bail!("{}", cli_error(kind, &message)),
|
||||
other => bail!("unexpected response: {other:?}"),
|
||||
@@ -2649,6 +2655,22 @@ fn cmd_redate_photos(
|
||||
}
|
||||
}
|
||||
|
||||
/// How a run `--wait` followed ended, as the exit status: an error when it
|
||||
/// failed, was cancelled, or left photos failed, so a script can tell.
|
||||
/// `outcome` is `(failed, cancelled, error)`; `None` when the daemon has no
|
||||
/// result, which happens when it restarted during the run.
|
||||
fn finished_run(what: &str, outcome: Option<(usize, bool, Option<String>)>) -> Result<()> {
|
||||
match outcome {
|
||||
None => bail!("{what} ended without a result; the service may have restarted"),
|
||||
Some((_, _, Some(error))) => bail!("{what} failed: {error}"),
|
||||
Some((_, true, None)) => bail!("{what} was cancelled"),
|
||||
Some((failed, false, None)) if failed > 0 => {
|
||||
bail!("{what} finished, but {failed} photo(s) failed")
|
||||
}
|
||||
Some(_) => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// A `YYYY-MM-DD` day as epoch seconds at UTC midnight. Reuses the Takeout
|
||||
/// scanner's date reader, which already accepts exactly this spelling.
|
||||
fn parse_day(day: &str) -> Result<i64> {
|
||||
@@ -2684,6 +2706,10 @@ fn print_redate_summary(summary: Option<&pdfs_core::control::RedateSummary>) {
|
||||
println!("No re-date has run yet.");
|
||||
return;
|
||||
};
|
||||
if let Some(error) = &summary.error {
|
||||
println!("Failed : {error}");
|
||||
return;
|
||||
}
|
||||
println!("Photos examined : {}", summary.examined);
|
||||
println!("Mis-dated : {}", summary.candidates);
|
||||
if summary.dry_run {
|
||||
@@ -2743,6 +2769,10 @@ fn print_import_summary(summary: Option<&pdfs_core::control::ImportSummary>) {
|
||||
println!("No import has run yet.");
|
||||
return;
|
||||
};
|
||||
if let Some(error) = &summary.error {
|
||||
println!("Failed : {error}");
|
||||
return;
|
||||
}
|
||||
println!("Photos in export : {}", summary.found);
|
||||
println!("Uploaded : {}", summary.uploaded);
|
||||
println!("Already on Proton: {}", summary.duplicates);
|
||||
@@ -3757,6 +3787,16 @@ mod login_wait_tests {
|
||||
assert_eq!(login_poll_delay(u32::MAX), Duration::from_secs(15));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_waited_for_run_that_did_not_fully_succeed_exits_with_an_error() {
|
||||
assert!(finished_run("the import", Some((0, false, None))).is_ok());
|
||||
assert!(finished_run("the import", Some((3, false, None))).is_err());
|
||||
assert!(finished_run("the import", Some((0, true, None))).is_err());
|
||||
let failed = finished_run("the import", Some((0, false, Some("not a zip".into()))));
|
||||
assert!(failed.unwrap_err().to_string().contains("not a zip"));
|
||||
assert!(finished_run("the import", None).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn waiting_for_a_session_stops_hammering_the_keyring() {
|
||||
let mut checks_left = 20;
|
||||
|
||||
@@ -1591,6 +1591,10 @@ pub struct RedateSummary {
|
||||
/// name implies)` — what a dry run is read for.
|
||||
#[serde(default)]
|
||||
pub samples: Vec<(String, i64, i64)>,
|
||||
/// Why the run as a whole failed, when it did. The counts are then
|
||||
/// empty: the run never got to the photos.
|
||||
#[serde(default)]
|
||||
pub error: Option<String>,
|
||||
}
|
||||
|
||||
/// How a Google Photos Takeout import is going, or how the last one ended
|
||||
@@ -1620,6 +1624,10 @@ pub struct ImportSummary {
|
||||
pub bytes: u64,
|
||||
/// True when the run stopped early because it was cancelled.
|
||||
pub cancelled: bool,
|
||||
/// Why the run as a whole failed, when it did, such as an archive that
|
||||
/// is not a zip. The counts are then empty.
|
||||
#[serde(default)]
|
||||
pub error: Option<String>,
|
||||
}
|
||||
|
||||
/// One album in a [`Response::Albums`] listing.
|
||||
|
||||
@@ -292,7 +292,14 @@ impl Core {
|
||||
let mut state = REDATE.lock();
|
||||
state.running = false;
|
||||
state.cancel = None;
|
||||
state.last = outcome.as_ref().ok().cloned();
|
||||
state.last = Some(match &outcome {
|
||||
Ok(summary) => summary.clone(),
|
||||
Err(e) => RedateSummary {
|
||||
error: Some(e.to_string()),
|
||||
dry_run,
|
||||
..Default::default()
|
||||
},
|
||||
});
|
||||
match &outcome {
|
||||
Ok(summary) => info!(
|
||||
candidates = summary.candidates,
|
||||
|
||||
@@ -544,7 +544,15 @@ impl Core {
|
||||
let mut state = IMPORT.lock();
|
||||
state.running = false;
|
||||
state.cancel = None;
|
||||
state.last = outcome.as_ref().ok().cloned();
|
||||
// A failed run is reported too: without it, `--wait` read
|
||||
// "no import has run" and exited as if all was well.
|
||||
state.last = Some(match &outcome {
|
||||
Ok(summary) => summary.clone(),
|
||||
Err(e) => ImportSummary {
|
||||
error: Some(e.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
});
|
||||
}
|
||||
match outcome {
|
||||
Ok(summary) => {
|
||||
|
||||
@@ -646,7 +646,12 @@ pub(crate) fn refresh_takeout(ui: &Rc<Ui>) {
|
||||
// An import runs for hours, so its ending is announced rather than
|
||||
// left to be discovered: a toast for whoever is looking at the
|
||||
// window, and a desktop notification for whoever is not.
|
||||
if was_running {
|
||||
let failed = summary.as_ref().and_then(|s| s.error.clone());
|
||||
if was_running && let Some(error) = &failed {
|
||||
let headline = gettext("Couldn't import from Google Photos");
|
||||
toast_error(&ui, &headline, error);
|
||||
notify("takeout-import", &headline, error);
|
||||
} else if was_running {
|
||||
let dry = ui.takeout.dry_run.get();
|
||||
let headline = if dry {
|
||||
gettext("Scan finished")
|
||||
@@ -702,7 +707,18 @@ fn repaint_summary(ui: &Rc<Ui>, summary: &ImportSummary) {
|
||||
} else {
|
||||
gettext("Last import")
|
||||
});
|
||||
ui.takeout.summary_group.set_description(Some(&if dry {
|
||||
ui.takeout.summary_group.set_description(Some(&if let Some(error) = &summary.error {
|
||||
// The reason is the service's own text, which is not translated. A
|
||||
// group's description is markup, and the reason may name a path.
|
||||
glib::markup_escape_text(&gettext_f(
|
||||
"{what}: {detail}",
|
||||
&[
|
||||
("what", &gettext("Couldn't import from Google Photos")),
|
||||
("detail", error),
|
||||
],
|
||||
))
|
||||
.to_string()
|
||||
} else if dry {
|
||||
gettext("Nothing was uploaded — this is what an import would do.")
|
||||
} else if summary.cancelled {
|
||||
gettext("Stopped early. Everything uploaded so far is in your timeline; run it again to continue where it left off.")
|
||||
|
||||
@@ -100,6 +100,7 @@ a live account since.
|
||||
|
||||
| Entry | Title |
|
||||
|---|---|
|
||||
| [B239](#b239----wait-reports-success-for-imports-and-re-dates-that-failed) | --wait reports success for imports and re-dates that failed |
|
||||
| [B238](#b238--draining-a-large-offline-batch-is-quadratic-in-its-size) | Draining a large offline batch is quadratic in its size |
|
||||
| [B237](#b237--every-reconnect-resends-refused-uploads-all-in-the-same-second) | Every reconnect resends refused uploads, all in the same second |
|
||||
| [B236](#b236--imported-re-dated-and-app-uploaded-photos-have-no-thumbnail) | Imported, re-dated and app-uploaded photos have no thumbnail |
|
||||
@@ -341,6 +342,32 @@ a live account since.
|
||||
|
||||
Newest first.
|
||||
|
||||
### B239 — --wait reports success for imports and re-dates that failed
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` section 3, CLI).
|
||||
**Where:** `finished_run`, `cmd_import_google_photos` and `cmd_redate_photos` in
|
||||
`crates/pdfs-cli/src/main.rs`; the run threads in `crates/pdfs-fuse/src/takeout.rs` and
|
||||
`redate.rs`; `ImportSummary::error` and `RedateSummary::error` in `crates/pdfs-core/src/control.rs`;
|
||||
`refresh_takeout` in `crates/pdfs-gui/src/app/pages/takeout.rs`
|
||||
|
||||
**Repro:** `pdfs import-google-photos --wait notes.txt; echo $?`, or an import where some photos
|
||||
fail to upload, or one cancelled from the app.
|
||||
|
||||
**Cause.** `--wait` exited 0 however the run ended. A run that failed as a whole (an archive that
|
||||
is not a zip) left no summary at all, so the CLI printed "No import has run yet" and the app
|
||||
announced "Import finished". A run with failed photos, or a cancelled one, also exited 0. A
|
||||
script could not tell any of these from a clean run.
|
||||
|
||||
**Fix.** A run that fails as a whole now leaves a summary with the reason. `--wait` exits 1 when
|
||||
the run failed, was cancelled, left photos failed, or ended without a result, and says which.
|
||||
The app shows the reason instead of "Import finished".
|
||||
|
||||
**Test:** `tests::a_waited_for_run_that_did_not_fully_succeed_exits_with_an_error` in
|
||||
`crates/pdfs-cli/src/main.rs`.
|
||||
|
||||
---
|
||||
|
||||
### B238 — Draining a large offline batch is quadratic in its size
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
|
||||
+683
-677
File diff suppressed because it is too large.
Load diff
+683
-680
File diff suppressed because it is too large.
Load diff
+683
-677
File diff suppressed because it is too large.
Load diff
Reference in new issue
Block a user