From a29c5e944c8f68ab5fa8011dc8cf0f8b28da3c02 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 7 Aug 2026 10:31:23 +0000 Subject: [PATCH] fix: address review nits on the ffi panic boundary Take the panic payload by `&(dyn Any + Send)` rather than `&Box<...>` (clippy::borrowed_box), stop claiming the hook always prints a backtrace, and correct the wide-number test comment, which still described the abort this change removes. Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-duckdb-ffi-internal/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/backend/windmill-duckdb-ffi-internal/src/lib.rs b/backend/windmill-duckdb-ffi-internal/src/lib.rs index f78c84db19..5385c6ff42 100644 --- a/backend/windmill-duckdb-ffi-internal/src/lib.rs +++ b/backend/windmill-duckdb-ffi-internal/src/lib.rs @@ -32,13 +32,14 @@ fn ffi_error(message: String) -> String { /// what the bindings themselves produce. fn catch_ffi_panic(what: &str, body: impl FnOnce() -> T) -> Result { catch_unwind(AssertUnwindSafe(body)).map_err(|payload| { - // The panic hook has already logged the message and backtrace to the - // worker's stderr; this only needs to reach the job's own error. - format!("panic in duckdb ffi {}: {}", what, panic_message(&payload)) + // The panic hook has already run, so the worker log carries the location + // (and a backtrace when RUST_BACKTRACE is on); this only has to reach the + // job's own error. + format!("panic in duckdb ffi {}: {}", what, panic_message(&*payload)) }) } -fn panic_message(payload: &Box) -> &str { +fn panic_message(payload: &(dyn Any + Send)) -> &str { payload .downcast_ref::<&str>() .copied() @@ -895,9 +896,9 @@ mod temporal_json_tests { } // Numbers too wide for a JSON number are rendered as strings. DECIMAL runs to - // 38 digits and UHUGEINT to 2^128-1; rendering either through a type that - // cannot hold it aborts the whole worker rather than erroring, because the - // panic escapes an `extern "C"` frame and those cannot unwind. + // 38 digits and UHUGEINT to 2^128-1; routing either through a type that cannot + // hold it panics deep in the bindings, which costs the job even though + // `catch_ffi_panic` keeps it from costing the worker. #[test] fn wide_numbers_render_as_strings_without_losing_precision() { let conn = duckdb::Connection::open_in_memory().unwrap();