From 2ac8d82ac438b4ac91c10f4c5affdb71f8af6402 Mon Sep 17 00:00:00 2001 From: Lukas Rieger Date: Sat, 19 Oct 2024 15:38:34 +0200 Subject: [PATCH 1/3] in debug, include location information in error message on panic --- godot-core/src/private.rs | 34 +++++++++++++------ .../src/object_tests/dynamic_call_test.rs | 14 ++++++-- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index 543c2b943..3a81d232e 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -387,27 +387,39 @@ where // Handle panic info only in Debug mode. #[cfg(debug_assertions)] { - let guard = info.lock().unwrap(); - let info = guard.as_ref().expect("no panic info available"); + let msg = extract_panic_message(err); + let mut msg = format_panic_message(msg); + + // try to add location information + if let Ok(guard) = info.lock() { + if let Some(info) = guard.as_ref() { + msg = format!("{}\n at {}:{}", msg, info.file, info.line); + } + } + if print { godot_error!( - "Rust function panicked at {}:{}.\n Context: {}", - info.file, - info.line, + "Rust function panicked: {}\n Context: {}", + msg, error_context() ); //eprintln!("Backtrace:\n{}", info.backtrace); } + + Err(msg) } - let msg = extract_panic_message(err); - let msg = format_panic_message(msg); + #[cfg(not(debug_assertions))] + { + let msg = extract_panic_message(err); + let msg = format_panic_message(msg); + + if print { + godot_error!("{msg}"); + } - if print { - godot_error!("{msg}"); + Err(msg) } - - Err(msg) } } } diff --git a/itest/rust/src/object_tests/dynamic_call_test.rs b/itest/rust/src/object_tests/dynamic_call_test.rs index 65a38a0f8..277a4912a 100644 --- a/itest/rust/src/object_tests/dynamic_call_test.rs +++ b/itest/rust/src/object_tests/dynamic_call_test.rs @@ -148,13 +148,21 @@ fn dynamic_call_with_panic() { assert_eq!(call_error.class_name(), Some("Object")); assert_eq!(call_error.method_name(), "call"); - assert_eq!( - call_error.to_string(), + + #[cfg(target_os = "windows")] + let path = "itest\\rust\\src\\object_tests\\object_test.rs"; + #[cfg(not(target_os = "windows"))] + let path = "itest/rust/src/object_tests/object_test.rs"; + + let expected_error_message = format!( "godot-rust function call failed: Object::call(&\"do_panic\")\ \n Source: ObjPayload::do_panic()\ - \n Reason: [panic] do_panic exploded" + \n Reason: [panic] do_panic exploded\ + \n at {path}:893" ); + assert_eq!(call_error.to_string(), expected_error_message); + obj.free(); } From 808b88da5605717ba31e5e934eca7504e09b896e Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Tue, 5 Nov 2024 20:27:00 +0100 Subject: [PATCH 2/3] Dynamically obtain line number of panic! expression --- godot-core/src/private.rs | 2 +- itest/rust/src/object_tests/dynamic_call_test.rs | 5 ++++- itest/rust/src/object_tests/object_test.rs | 5 +++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index 3a81d232e..9ac5c4f50 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -390,7 +390,7 @@ where let msg = extract_panic_message(err); let mut msg = format_panic_message(msg); - // try to add location information + // Try to add location information. if let Ok(guard) = info.lock() { if let Some(info) = guard.as_ref() { msg = format!("{}\n at {}:{}", msg, info.file, info.line); diff --git a/itest/rust/src/object_tests/dynamic_call_test.rs b/itest/rust/src/object_tests/dynamic_call_test.rs index 277a4912a..64205f552 100644 --- a/itest/rust/src/object_tests/dynamic_call_test.rs +++ b/itest/rust/src/object_tests/dynamic_call_test.rs @@ -154,11 +154,14 @@ fn dynamic_call_with_panic() { #[cfg(not(target_os = "windows"))] let path = "itest/rust/src/object_tests/object_test.rs"; + // Obtain line number dynamically, avoids tedious maintenance on code reorganization. + let line = ObjPayload::get_panic_line(); + let expected_error_message = format!( "godot-rust function call failed: Object::call(&\"do_panic\")\ \n Source: ObjPayload::do_panic()\ \n Reason: [panic] do_panic exploded\ - \n at {path}:893" + \n at {path}:{line}" ); assert_eq!(call_error.to_string(), expected_error_message); diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs index a116573a8..93a25a3cc 100644 --- a/itest/rust/src/object_tests/object_test.rs +++ b/itest/rust/src/object_tests/object_test.rs @@ -887,6 +887,11 @@ impl ObjPayload { fn do_panic(&self) { panic!("do_panic exploded"); } + + // Obtain the line number of the panic!() call above; keep equidistant to do_panic() method. + pub fn get_panic_line() -> u32 { + line!() - 5 + } } // ---------------------------------------------------------------------------------------------------------------------------------------------- From 6eaae5701e805fe697d255b580e6e58c17bc403a Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Tue, 5 Nov 2024 21:06:44 +0100 Subject: [PATCH 3/3] Handle different string in Release mode --- .../src/object_tests/dynamic_call_test.rs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/itest/rust/src/object_tests/dynamic_call_test.rs b/itest/rust/src/object_tests/dynamic_call_test.rs index 64205f552..6b4f960af 100644 --- a/itest/rust/src/object_tests/dynamic_call_test.rs +++ b/itest/rust/src/object_tests/dynamic_call_test.rs @@ -149,19 +149,25 @@ fn dynamic_call_with_panic() { assert_eq!(call_error.class_name(), Some("Object")); assert_eq!(call_error.method_name(), "call"); - #[cfg(target_os = "windows")] - let path = "itest\\rust\\src\\object_tests\\object_test.rs"; - #[cfg(not(target_os = "windows"))] - let path = "itest/rust/src/object_tests/object_test.rs"; + let appendix = if cfg!(debug_assertions) { + let mut path = "itest/rust/src/object_tests/object_test.rs".to_string(); - // Obtain line number dynamically, avoids tedious maintenance on code reorganization. - let line = ObjPayload::get_panic_line(); + if cfg!(target_os = "windows") { + path = path.replace('/', "\\") + } + + // Obtain line number dynamically, avoids tedious maintenance on code reorganization. + let line = ObjPayload::get_panic_line(); + + format!("\n at {path}:{line}") + } else { + String::new() + }; let expected_error_message = format!( "godot-rust function call failed: Object::call(&\"do_panic\")\ \n Source: ObjPayload::do_panic()\ - \n Reason: [panic] do_panic exploded\ - \n at {path}:{line}" + \n Reason: [panic] do_panic exploded{appendix}" ); assert_eq!(call_error.to_string(), expected_error_message);