Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

On panic in a godot-rust call, location information is thrown away and not shown #923

Closed
0x53A opened this issue Oct 20, 2024 · 1 comment · Fixed by #926
Closed

On panic in a godot-rust call, location information is thrown away and not shown #923

0x53A opened this issue Oct 20, 2024 · 1 comment · Fixed by #926
Labels
bug c: core Core components

Comments

@0x53A
Copy link
Contributor

0x53A commented Oct 20, 2024

use godot::prelude::*;

use godot::classes::Node;
use godot::classes::INode;

#[derive(GodotClass)]
#[class(base=Node, init)]
pub struct ErroringNode {
    base: Base<Node>
}


#[godot_api]
impl INode for ErroringNode {

    fn ready(&mut self) {
        let o : Option<i32> = None;
        let _ = o.unwrap();
    }
}

This panics immediatly at startup, when added to a scene. The panic is handled and printed to the error log.

Unfortunately, the way the panic is currently handled, location information is not printed to the console, making it potentially hard to find.

Before:

ERROR: godot-rust function call failed: ErroringNode::ready()
    Reason: [panic]  called `Option::unwrap()` on a `None` value
   at: godot_core::private::report_call_error (C:\Users\<user>\.cargo\git\checkouts\gdext-4542fb3ebbcdf22f\a0d5799\godot-core\src\private.rs:332)

For now I patched this in a fork: a0d5799...a9bf8dc

In this case, extract_panic_message only returns called `Option::unwrap()` on a `None` value. Location information is only printed to the console if print were true, which it is not, and not returned back from the function.

After:

ERROR: godot-rust function call failed: ErroringNode::ready()
    Reason: Rust function panicked at src\ErroringNode.rs:18.
[panic]  called `Option::unwrap()` on a `None` value
  Context: ErroringNode::ready
   at: godot_core::private::report_call_error (C:\Users\<user>\.cargo\git\checkouts\gdext-4542fb3ebbcdf22f\a9bf8dc\godot)

I can send my hack as a PR, as is, but there might also be a worthwhile discussion whether this should be linked debug/release compiler configuration, as it currently stands, or configurable at runtime.

If I have a crash reporting system, location information would be extremely valuable telemetry, even (or especially) for release builds.

@Bromeon
Copy link
Member

Bromeon commented Oct 20, 2024

Thanks! A pull request that fixes this in Debug mode would be appreciated 🙂

but there might also be a worthwhile discussion whether this should be linked debug/release compiler configuration, as it currently stands, or configurable at runtime.

This was recently introduced due to performance concerns: #889.

What are the arguments for having it more configurable? It's not easy to strike a good balance between customizability and complexity. There's also no dedicated config API yet, so a lot of things end up in ExtensionLibrary, or worse, as Cargo features...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug c: core Core components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants