From 7e1c4ab3187d0c6acf597f1ccde4eef0989cbfa9 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 10:41:09 -0400 Subject: [PATCH] backfill_date_taken: surface the actual diesel error in warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DAO swallowed every diesel::update failure as a flat `anyhow!("Update error")`, then trace_db_call further reduced it to `DbError { kind: UpdateError }`. Operators saw "update failed for lib 2 Snapchat/foo.mp4: DbError { kind: UpdateError }" with no clue why (constraint violation? type mismatch? row vanished mid-flight? DB locked?). Two changes: - Preserve the diesel error in the anyhow chain along with the input params (lib, rel_path, date_taken, source) so the cause is visible. - Log the chain at warn-level inside the DAO before the trace wrapper collapses it to DbErrorKind::UpdateError, so the warning at the call site finally has something diagnosable next to it. - Treat zero-row updates as a debug-level "row likely retired by the missing-file scan" rather than a hard failure — that case is benign and shouldn't poison the drain's error tally. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/database/mod.rs | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/database/mod.rs b/src/database/mod.rs index eeb457e..b275988 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -1198,17 +1198,52 @@ impl ExifDao for SqliteExifDao { let mut connection = self.connection.lock().expect("Unable to get ExifDao"); - diesel::update( + let result = diesel::update( image_exif .filter(library_id.eq(library_id_val)) .filter(rel_path.eq(rel_path_val)), ) .set((date_taken.eq(date_taken_val), date_taken_source.eq(source))) - .execute(connection.deref_mut()) - .map(|_| ()) - .map_err(|_| anyhow::anyhow!("Update error")) + .execute(connection.deref_mut()); + + match result { + Ok(rows) => { + // Surface zero-row updates as a warning rather than a + // silent success. They mean the (library_id, rel_path) + // row was deleted between the `get_rows_needing_date_ + // backfill` query and this update — rare but possible + // when the file watcher is racing the drain. The drain + // shouldn't treat that as a hard error, so still + // return Ok(()). + if rows == 0 { + log::debug!( + "backfill_date_taken: 0 rows matched lib={} {} \ + (row likely retired by missing-file scan)", + library_id_val, + rel_path_val + ); + } + Ok(()) + } + // Preserve the diesel error in the chain so warnings at + // the call site can articulate the cause (a flat "Update + // error" was useless for triage). + Err(e) => Err(anyhow::anyhow!( + "diesel update failed (lib={}, rel_path={}, date_taken={}, source={}): {}", + library_id_val, + rel_path_val, + date_taken_val, + source, + e + )), + } + }) + .map_err(|e| { + // Log before the anyhow message gets stripped by the + // DbError-only return type. + log::warn!("backfill_date_taken: {}", e); + DbError::new(DbErrorKind::UpdateError) }) - .map_err(|_| DbError::new(DbErrorKind::UpdateError)) } fn set_manual_date_taken(