Add custom error handling option #88
Conversation
|
The only thing I really dislike is random numbers we have to provide as the |
| @@ -34,7 +35,7 @@ program test_micm_fort_api | |||
|
|
|||
|
|
|||
| write(*,*) "[test micm fort api] Creating MICM solver..." | |||
| micm => micm_t(config_path, errcode) | |||
| micm => micm_t(config_path) | |||
There was a problem hiding this comment.
There's no recourse here for when creating micm fails. The error function is called, I think, but the program still thinks that micm is valid.
src/micm/micm.cpp
Outdated
| return micm; | ||
| } catch (const std::exception &e) { | ||
| delete micm; | ||
| Error(909039518, std::string("Error creating MICM solver: " + std::string(e.what())).c_str()); |
There was a problem hiding this comment.
Since these codes are really meant to make where the error happened identifiable, maybe this is another time where a macro is useful and we use __LINE__ and __FILE__ to do this instead?
Yeah, I agree. After more discussion and checking with the CAM-SIMA developers, I think I might redo this PR to just have the MUSICA C API functions always return an error code and message by catching any exceptions thrown in MICM. I will also explore options that don't require us to create random numbers for error codes. |
|
I think this is ready for another look now. All the API functions return an error object that has a code, category, and message. The combination of the code and category can be used to identify specific errors, and there are some helper functions to do this. Let me know what you think! |
There was a problem hiding this comment.
Looks great @mattldawson!
Had two suggestions to use base functions in the error handler that might be worth while but nothing that should hold up the PR.
src/util.cpp
Outdated
| Error NoError() | ||
| { | ||
| Error error; | ||
| error.code_ = 0; | ||
| error.category_ = ToConstString(""); | ||
| error.message_ = ToConstString("Success"); | ||
| return error; | ||
| } |
There was a problem hiding this comment.
| Error NoError() | |
| { | |
| Error error; | |
| error.code_ = 0; | |
| error.category_ = ToConstString(""); | |
| error.message_ = ToConstString("Success"); | |
| return error; | |
| } | |
| Error NoError() | |
| { | |
| return ToError(ToConstString(""), 0, ToConstString("Success")); | |
| } |
src/util.cpp
Outdated
| Error ToError(const std::system_error& e) | ||
| { | ||
| Error error; | ||
| error.code_ = e.code().value(); | ||
| error.category_ = ToConstString(e.code().category().name()); | ||
| error.message_ = ToConstString(e.what()); | ||
| return error; | ||
| } |
There was a problem hiding this comment.
| Error ToError(const std::system_error& e) | |
| { | |
| Error error; | |
| error.code_ = e.code().value(); | |
| error.category_ = ToConstString(e.code().category().name()); | |
| error.message_ = ToConstString(e.what()); | |
| return error; | |
| } | |
| Error ToError(const std::system_error& e) | |
| { | |
| return ToError(ToConstString(e.code().category().name()), | |
| e.code().value(), | |
| ToConstString(e.what())); |
Adds an option for users to provide a custom error handling function through the C API that will be called for any exceptions thrown by the C++ libraries.
closes #38
I'm not sure if this is the best design. I think it will work for the CAM-SIMA use case where they just want to provide a single function that gets called for any error. But, I'm not sure if a global function pointer for errors is the best solution. I don't mind changing this approach entirely if anyone has a better solution.