From 3427c2916c368b123aef342fb2264c7338542138 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Tue, 12 May 2026 14:37:26 -0400 Subject: [PATCH] Log 500-return paths in PATCH /image/faces/{id} The four 500-return paths in update_face_handler returned e.to_string() in the body but never logged. When a face PATCH failed with a 16-byte body and no log entry, the cause (SQLITE_BUSY from cross-DAO writer contention exhausting the 5s busy_timeout) was invisible. Surface the full anyhow chain via {:#} on each path so the diesel cause is in the log even when the response body only shows the top-level context. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/faces.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/faces.rs b/src/faces.rs index c81a270..f4bd5cc 100644 --- a/src/faces.rs +++ b/src/faces.rs @@ -2018,12 +2018,19 @@ async fn update_face_handler( match dao.get_face(&span_context, id) { Ok(Some(r)) => r, Ok(None) => return HttpResponse::NotFound().finish(), - Err(e) => return HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => { + warn!("PATCH /image/faces/{}: 500 — get_face failed: {:#}", id, e); + return HttpResponse::InternalServerError().body(e.to_string()); + } } }; let library = match app_state.library_by_id(current.library_id) { Some(l) => l.clone(), None => { + warn!( + "PATCH /image/faces/{}: 500 — face row references unknown library_id {}", + id, current.library_id + ); return HttpResponse::InternalServerError().body(format!( "face row references unknown library_id {}", current.library_id @@ -2106,7 +2113,14 @@ async fn update_face_handler( let mut dao = face_dao.lock().expect("face dao lock"); let row = match dao.update_face(&span_context, id, person_patch, bbox_patch, new_embedding) { Ok(r) => r, - Err(e) => return HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => { + // The full anyhow chain (`{:#}`) shows the diesel cause behind + // the short context string we surface in the response body — + // SQLITE_BUSY here usually means another DAO's writer held the + // lock past `busy_timeout` (5s), which is invisible in `{}`. + warn!("PATCH /image/faces/{}: 500 — update_face failed: {:#}", id, e); + return HttpResponse::InternalServerError().body(e.to_string()); + } }; // Hydrate person_name so the response shape matches GET /image/faces // — the carousel overlay does an optimistic replace on this row, and @@ -2114,7 +2128,13 @@ async fn update_face_handler( // VFD label off the bbox even though the assignment didn't change. match hydrate_face_with_person(&mut *dao, &span_context, row) { Ok(joined) => HttpResponse::Ok().json(joined), - Err(e) => HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => { + warn!( + "PATCH /image/faces/{}: 500 — hydrate_face_with_person failed: {:#}", + id, e + ); + HttpResponse::InternalServerError().body(e.to_string()) + } } }