Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

remove exception propagation for callbacks #118

Merged
merged 1 commit into from
Nov 21, 2023
Merged

remove exception propagation for callbacks #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions ortools/linear_solver/xpress_interface.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,22 +263,26 @@ class MPCallbackWrapper {
explicit MPCallbackWrapper(MPCallback* callback) : callback_(callback){};
MPCallback* GetCallback() const { return callback_; }
// Since our (C++) call-back functions are called from the XPRESS (C) code,
// exceptions thrown in our call-back code are not caught by XPRESS.
// We have to catch them, interrupt XPRESS, and re-throw them after XPRESS is
// exceptions thrown in our call-back code are not caught by XPRESS.
// We have to catch them, interrupt XPRESS, and log them after XPRESS is
// effectively interrupted (ie after solve).
void CatchException(XPRSprob cbprob) {
exceptions_mutex_.lock();
caught_exceptions_.push_back(std::current_exception());
interruptXPRESS(cbprob, CALLBACK_EXCEPTION);
exceptions_mutex_.unlock();
}
void RethrowCaughtExceptions() {
void LogCaughtExceptions() {
exceptions_mutex_.lock();
for (const std::exception_ptr& ex : caught_exceptions_) {
try {
std::rethrow_exception(ex);
} catch (std::bad_exception const&) {
LOG(ERROR) << "Bad exception";
} catch (std::exception &ex) {
// We don't want the interface to throw exceptions, plus it causes
// SWIG issues in Java & Python. Instead, we'll only log them.
// (The use cases where the user has to raise an exception inside their
// call-back does not seem to be frequent, anyway.)
LOG(ERROR) << "Caught exception during user-defined call-back: " << ex.what();
}
}
caught_exceptions_.clear();
Expand Down Expand Up @@ -1904,7 +1908,7 @@ MPSolver::ResultStatus XpressInterface::Solve(MPSolverParameters const& param) {
}

if (mp_callback_wrapper != nullptr) {
mp_callback_wrapper->RethrowCaughtExceptions();
mp_callback_wrapper->LogCaughtExceptions();
delete mp_callback_wrapper;
}

Expand Down
19 changes: 7 additions & 12 deletions ortools/linear_solver/xpress_interface_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1333,22 +1333,17 @@ TEST(XpressInterface, SetAndResetCallBack) {
}

TEST(XpressInterface, CallbackThrowsException) {
// Test that when the callback throws an exception, it is caught and re-thrown
// Test that when the callback throws an exception, it is caught and logged
UNITTEST_INIT_MIP();
auto oldMpCallback = buildLargeMipWithCallback(solver, 30, 30);
auto newMpCallback = new MyMPCallback(&solver, true);
solver.SetCallback((MPCallback*)newMpCallback);
EXPECT_THROW(
{
try {
solver.Solve();
} catch (const std::runtime_error& e) {
// this tests that it has the correct message
EXPECT_STREQ("This is a mocked exception in MyMPCallback", e.what());
throw;
}
},
std::runtime_error);
testing::internal::CaptureStderr();
EXPECT_NO_THROW(solver.Solve());
std::string errors = testing::internal::GetCapturedStderr();
// Test that StdErr contains the following error message
std::string expected_error = "Caught exception during user-defined call-back: This is a mocked exception in MyMPCallback";
ASSERT_NE(errors.find(expected_error), std::string::npos);
}

} // namespace operations_research
Expand Down
Loading