handle errors

wjones127 · Dec 27, 2023 · ed3eba3 · ed3eba3
1 parent 6acd2f8
commit ed3eba3
Showing 1 changed file with 30 additions and 24 deletions.
diff --git a/python/pyarrow/src/arrow/python/common.cc b/python/pyarrow/src/arrow/python/common.cc
@@ -89,31 +89,11 @@ class PythonErrorDetail : public StatusDetail {
   const char* type_id() const override { return kErrorDetailTypeId; }
 
   std::string ToString() const override {
-    PyAcquireGIL lock;
-
-    // Use traceback.format_exception()
-    OwnedRef traceback_module;
-    // TODO: what to do with Status?
-    internal::ImportModule("traceback", &traceback_module);
+    // This is simple enough not to need the GIL
+    Result<std::string> result = FormatImpl();
 
-    OwnedRef fmt_exception;
-    internal::ImportFromModule(traceback_module.obj(), "format_exception",
-                               &fmt_exception);
-
-    OwnedRef formatted;
-    formatted.reset(PyObject_CallFunctionObjArgs(fmt_exception.obj(), exc_type_.obj(),
-                                                 exc_value_.obj(), exc_traceback_.obj(),
-                                                 NULL));
-
-    if (formatted) {
-      std::string result = "Python exception: ";
-      Py_ssize_t num_lines = PyList_GET_SIZE(formatted.obj());
-      for (Py_ssize_t i = 0; i < num_lines; ++i) {
-        std::string line_str;
-        internal::PyObject_StdStringStr(PyList_GET_ITEM(formatted.obj(), i), &line_str);
-        result += line_str;
-      }
-      return result;
+    if (result.ok()) {
+      return result.ValueOrDie();
     } else {
       // Fallback to just the exception type
       const auto ty = reinterpret_cast<const PyTypeObject*>(exc_type_.obj());
@@ -157,6 +137,32 @@ class PythonErrorDetail : public StatusDetail {
   }
 
  protected:
+  Result<std::string> FormatImpl() const {
+    PyAcquireGIL lock;
+
+    // Use traceback.format_exception()
+    OwnedRef traceback_module;
+    RETURN_NOT_OK(internal::ImportModule("traceback", &traceback_module));
+
+    OwnedRef fmt_exception;
+    RETURN_NOT_OK(internal::ImportFromModule(traceback_module.obj(), "format_exception",
+                                             &fmt_exception));
+
+    OwnedRef formatted;
+    formatted.reset(PyObject_CallFunctionObjArgs(fmt_exception.obj(), exc_type_.obj(),
+                                                 exc_value_.obj(), exc_traceback_.obj(),
+                                                 NULL));
+
+    std::string result = "Python exception: ";
+    Py_ssize_t num_lines = PyList_GET_SIZE(formatted.obj());
+    for (Py_ssize_t i = 0; i < num_lines; ++i) {
+      std::string line_str;
+      internal::PyObject_StdStringStr(PyList_GET_ITEM(formatted.obj(), i), &line_str);
+      result += line_str;
+    }
+    return result;
+  }
+
   PythonErrorDetail() = default;
 
   OwnedRefNoGIL exc_type_, exc_value_, exc_traceback_;