From 5892a01661107f1182e839f0ca230910fa1020d4 Mon Sep 17 00:00:00 2001
From: John Turner <7strbass@gmail.com>
Date: Fri, 30 Jun 2023 10:14:38 -0400
Subject: [PATCH] --modify ESP_CHECK  to exit not abort.

---
 src/esp/bindings/Bindings.cpp | 10 ++++++----
 src/esp/core/Check.cpp        | 30 +++++++++++++++---------------
 src/esp/core/Check.h          | 19 ++++++++++---------
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/src/esp/bindings/Bindings.cpp b/src/esp/bindings/Bindings.cpp
index f168ec3cc1..5a403b3289 100644
--- a/src/esp/bindings/Bindings.cpp
+++ b/src/esp/bindings/Bindings.cpp
@@ -45,10 +45,12 @@ PYBIND11_MODULE(habitat_sim_bindings, m) {
       false;
 #endif
 
-  /* This function pointer is used by ESP_CHECK(). If it's null, it
-     std::abort()s, if not, it calls it to cause a Python AssertionError */
-  esp::core::throwInPython = [](const char* const message) {
-    PyErr_SetString(PyExc_AssertionError, message);
+  /**
+   * This function pointer is used by ESP_CHECK(). If this is null, the macro
+   * std::exit(1)s, if not, the macro calls this to throw a Python RuntimeError
+   */
+  esp::core::throwRuntimeInPython = [](const char* const message) {
+    PyErr_SetString(PyExc_RuntimeError, message);
     throw pybind11::error_already_set{};
   };
 
diff --git a/src/esp/core/Check.cpp b/src/esp/core/Check.cpp
index d914040e33..b53cae18b8 100644
--- a/src/esp/core/Check.cpp
+++ b/src/esp/core/Check.cpp
@@ -9,28 +9,28 @@
 namespace esp {
 namespace core {
 
-void (*throwInPython)(const char*) = nullptr;
+void (*throwRuntimeInPython)(const char*) = nullptr;
 
 /* [[noreturn]] will help the compiler optimize -- it basically tells it that
    the condition passed to HABITAT_EXCEPTION() can be assumed to be always true
    in the following code, because if not then the execution ends in this
    function. */
-[[noreturn]] void throwIfInPythonOtherwiseAbort(const char* message) {
-  /* The throwInPython function pointer gets filled during Python bindings
-     startup. If it's nullptr, we're in plain C++ code. */
-  if (throwInPython) {
-    throwInPython(message);
-    /* I failed to apply the NORETURN attribute to the throwInPython function
-       pointer so at least this */
-    CORRADE_INTERNAL_ASSERT_UNREACHABLE();
+
+[[noreturn]] void throwIfInPythonOtherwiseExit(const char* message) {
+  /* The throwRuntimeInPython function pointer gets filled during Python
+     bindings startup. If it's nullptr, we're in plain C++ code. */
+  if (throwRuntimeInPython) {
+    throwRuntimeInPython(message);
+
+    std::exit(1);
   }
 
-  /* If it isn't defined, do an abort the same way as CORRADE_ASSERT(). This
-     could be in an `else` block but I like to play with fire, so it's not --
-     the NORETURN above should tell that to the compiler and the function
-     should throw. */
-  Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << message;
-  std::abort();
+  /*
+   * If it isn't defined, display a Fatal message, which will terminate with
+   * std::exit(1).
+   */
+  Corrade::Utility::Fatal{Corrade::Utility::Fatal::defaultOutput(), 1}
+      << message;
 }
 
 }  // namespace core
diff --git a/src/esp/core/Check.h b/src/esp/core/Check.h
index f15acf221f..1c01f42d57 100644
--- a/src/esp/core/Check.h
+++ b/src/esp/core/Check.h
@@ -12,9 +12,9 @@
 /** @file
   @brief ESP_CHECK macro, for use with fatal runtime errors.
 
-  Below is an overview of asserts, ESP_CHECK, exception-throwing, and warnings
-  in Habitat-sim. These are new guidelines as of Feb 2021; not all Habitat code
-  follows these guidelines yet.
+  Below is an overview of asserts, ESP_CHECK vs. ESP_FATAL, exception-throwing,
+  and warnings in Habitat-sim. These are new guidelines as of Feb 2021, and
+  updated June 2023; not all Habitat code follows these guidelines yet.
 
   assert
   - see CORRADE_ASSERT and CORRADE_ASSERT_INTERNAL.
@@ -51,18 +51,19 @@
 namespace esp {
 namespace core {
 
-/* The throwInPython function pointer gets filled during Python bindings
+/* The throwRuntimeInPython function pointer gets filled during Python bindings
    startup. If it's nullptr, we're in plain C++ code. */
-extern void (*throwInPython)(const char*);
+extern void (*throwRuntimeInPython)(const char*);
 
 // For use in ESP_CHECK
-[[noreturn]] void throwIfInPythonOtherwiseAbort(const char* message);
+[[noreturn]] void throwIfInPythonOtherwiseExit(const char* message);
 
 }  // namespace core
 }  // namespace esp
 
-/* A runtime check that must pass, otherwise we consider this a fatal runtime
-error. The program terminates with the supplied error message. */
+/* A runtime check that must pass, otherwise we consider this a fatal
+ * user-caused error (bad data). The program exist with the supplied error
+ * message but no core dump. */
 #define ESP_CHECK(condition, ...)                                 \
   do {                                                            \
     if (!(condition)) {                                           \
@@ -70,7 +71,7 @@ error. The program terminates with the supplied error message. */
       Corrade::Utility::Debug{                                    \
           &out, Corrade::Utility::Debug::Flag::NoNewlineAtTheEnd} \
           << "ESP_CHECK failed:" << __VA_ARGS__;                  \
-      esp::core::throwIfInPythonOtherwiseAbort(out.str().data()); \
+      esp::core::throwIfInPythonOtherwiseExit(out.str().data());  \
     }                                                             \
   } while (false)