nomad-software / tkd

GUI toolkit for the D programming language based on Tcl/Tk
MIT License
117 stars 16 forks source link

commandCallbackHandler catches Throwable making unrecoverable Errors impossible #43

Closed marrub-- closed 7 years ago

marrub-- commented 7 years ago

In this code:

catch (Throwable ex)
{
    string error = "Error occurred in command callback. ";
    error ~= ex.msg ~ "\n";
    error ~= "Element: " ~ args.element.id ~ "\n";

    Tcl_SetResult(tclInterpreter, error.toStringz, TCL_STATIC);
    return TCL_ERROR;
}

Instead of catching Exception, it catches Throwable, making erroneous states that cannot logically be recovered from impossible. If the program throws an Error it will continue, even if this Error was caused by memory corruption or other such unrecoverable errata. It should instead catch Exception which is reserved for errors that are safe to catch and handle (and therefore continue or recover from.)

nomad-software commented 7 years ago

I must have used Throwable here for a reason (it's been three years, lol) probably to indeed catch memory errors. IIRC if an error is caught here, the program will crash and show you a Tk dialog box with the problem. This is built-in behaviour in Tcl/Tk.

marrub-- commented 7 years ago

The program continues execution as normal after displaying an error box. Not being able to crash the application is, surprisingly, a nuisance. :stuck_out_tongue:

The issue being that I have an external exception handler which needs to run instead of Tk's if an error occurs. Having it always catch everything is a big problem in this case.

nomad-software commented 7 years ago

Have you got a small example I could look at please?

marrub-- commented 7 years ago

Sure, here. The program never crashes despite throwing an Error. I think this could be okay if an option to rethrow on callback errors were available, though I don't know how exactly that would be implemented.

nomad-software commented 7 years ago

I've done a lot of testing of different solutions and I think just changing Throwable to Exception is the simplest fix. Tkd should just let the program crash on encountering an error in a command. Done in commit: https://github.com/nomad-software/tkd/commit/35e1d91c24a73a0abd0ca8cbac017e4694ae49ff

I've also created a new release for this too: https://github.com/nomad-software/tkd/releases/tag/v1.1.10