Error handling {c,sh}ould be improved

[dpaetzel]

Disclaimer: Solving this may be quite a pain and time consuming and not much fun.

I often stumble over the fact that the C code calls exit(EXIT_FAILURE). I understand that this is probably for historic reasons (Python bindings were added later) but this regularly kills my interactive Python (or Julia) session (i.e. instead of an exception being thrown, the entire session ends). In Python this isn't that bad, because you can start a new session rather quickly (if you keep a scratch pad of what you entered nearby—or use Jupyter or something, I guess) but for Julia this is quite unfortunate because restarting a session can take quite a few seconds due to precompilation etc.

What would need to be changed? If the current code style (mostly C) should be kept, then all exit(…)s would need to be replaced, e.g. by the surrounding function returning an int status instead of void and that would have to be propagated up to 1) main (for pure C stuff) and 2) the Python bindings. There the error could be rethrown either as exit(…) or using pybind exceptions. Maybe this can be done simpler by switching to C++ features but that would still require many changes, I guess.

I just thought I'd report this, because it's a regularly occurring problem for me. Feel free to close for reasons of “no intent to support this”. 🙂

This may also have been a topic of discussion somewhere in this repo but I couldn't find a proper writeup.

[rpreen]

Yeah, it is a significant annoyance for things like running in Jupyter notebooks. I did once look at trying to fix this, but it was more complicated than I thought and I couldn't see a nice solution. C++ and Pybind work much better together and you can just throw C++ exceptions that automatically get picked up in Python.

I remember I did try several things to wrap or replace the exit function to be able to throw exceptions for C++/Python builds, such as the ugly catch_error function:

xcsf/xcsf/utils.h

Lines 129 to 140 in 07fb6f0

	/**
	* @brief Catches parameter value errors.
	* @param [in] ret String return type from JSON import.
	*/
	static inline void
	catch_error(const char *ret)
	{
	if (ret != NULL) {
	printf("%s\n", ret);
	exit(EXIT_FAILURE);
	}
	}

which then wraps things like parameter setters that return a string message if an error occurred (or NULL if fine)

catch_error(param_set_alpha(xcsf, json->valuedouble));

const char *
param_set_alpha(struct XCSF *xcsf, const double a)
{
    if (a < 0 || a > 1) {
        return "Invalid ALPHA. Range: [0,1]";
    }
    xcsf->ALPHA = a;
    return NULL;
}     

and the idea was to then #ifdef a different function that throws an exception for C++ builds - I can't remember exactly what/why it failed, I think it required other major changes including the build process.

I really hate C++, but I guess I don't really care if the build moves entirely to C++ if it's just adding throwing exceptions so that the rest of the code base remains in clean C.

[dpaetzel]

I'm not at all a fan of changing everything to C++. Another minimally invasive idea may be (full disclosure: this was recommended by Claude, I hope no one gets angry about that 😇 ) to have a global error handler that can be set differently for the pure C and pybind parts like this?

// C library
typedef void (*error_callback_t)(int code, const char* msg);
static error_callback_t error_handler = default_exit_handler;

void library_set_error_handler(error_callback_t handler) {
    error_handler = handler;
}

void some_void_function(data_t* data) {
    if (error_condition) {
        error_handler(ERROR_CODE, "something failed");
        return;  // handler decides what happens
    }
    // normal processing
}

static void default_exit_handler(int code, const char* msg) {
    fprintf(stderr, "Error: %s\n", msg);
    exit(code);  // original behavior
}

This would only require to change all calls to exit to calls to error_handler.

The pybind part could then define:

#include <pybind11/pybind11.h>

void python_error_handler(int code, const char* msg) {
    throw std::runtime_error(std::string("C library error: ") + msg);
}

PYBIND11_MODULE(mymodule, m) {
    // Set custom handler during module initialization
    library_set_error_handler(python_error_handler);
    
    m.def("some_function", &some_void_function);
}