Merge pull request #1617 from rstudio/refactor-exceptions

Convert Python Exceptions to R conditions that are R lists, not R envs.

NAMESPACE

@@ -41,8 +41,6 @@ S3method(as.double,numpy.ndarray)
 S3method(as.environment,python.builtin.object)
 S3method(as.matrix,numpy.ndarray)
 S3method(as.vector,numpy.ndarray)
-S3method(conditionCall,python.builtin.BaseException)
-S3method(conditionMessage,python.builtin.BaseException)
 S3method(dim,pandas.core.frame.DataFrame)
 S3method(dim,pandas.core.series.Series)
 S3method(dim,scipy.sparse._base._spbase)

NEWS.md

@@ -3,6 +3,10 @@
 - Fixed an issue where attempting to convert a non-simple NumPy array
   to R would signal an error. (#1613, fixed in #1614).
 
+- Python Exceptions converted to R conditions are now R lists instead
+  of R environments, for compatability with {rlang} and {purrr}.
+  (tidyverse/purrr#1104, r-lib/rlang#1664, #1617)
+
 # reticulate 1.37.0
 
 - Interrupting Python no longer leads to segfaults.

R/utils.R

@@ -162,23 +162,18 @@ r_to_py.error <- function(x, convert = FALSE) {
   e
 }
 
-#' @export
-conditionCall.python.builtin.BaseException <- function(c) {
-  as_r_value(py_get_attr(c, "call", TRUE))
-}
-
-#' @export
-conditionMessage.python.builtin.BaseException <- function(c) {
-  conditionMessage_from_py_exception(c)
-}
-
 
 #' @export
 `$.python.builtin.BaseException` <- function(x, name) {
-  if(identical(name, "call"))
-    return(conditionCall(x))
-  if(identical(name, "message"))
-    return(conditionMessage(x))
+  if(identical(name, "call")) {
+    out <- if(typeof(x) == "list") unclass(x)[["call"]]
+    return(out %||% as_r_value(py_get_attr(x, "call", TRUE)))
+  }
+
+  if(identical(name, "message")) {
+    out <- if(typeof(x) == "list") unclass(x)[["message"]]
+    return(out %||% conditionMessage_from_py_exception(x))
+  }
 
   py_maybe_convert(py_get_attr(x, name, TRUE), py_has_convert(x))
 }

src/python.cpp

@@ -606,7 +606,7 @@ std::string as_r_class(PyObject* classPtr) {
 
 }
 
-SEXP py_class_names(PyObject* object) {
+SEXP py_class_names(PyObject* object, bool exception) {
 
   // Py_TYPE() usually returns a borrowed reference to object.__class__
   // but can differ if __class__ was modified after the object was created.
@@ -666,7 +666,7 @@ SEXP py_class_names(PyObject* object) {
     classNames.insert(classNames.end() - 1, "python.builtin.iterator");
 
   // if it's a BaseException instance, append "error"/"interrupt" and "condition"
-  if (PyExceptionInstance_Check(object)) {
+  if (exception) {
     if (PyErr_GivenExceptionMatches(type, PyExc_KeyboardInterrupt))
       classNames.push_back("interrupt");
     else
@@ -675,7 +675,13 @@ SEXP py_class_names(PyObject* object) {
   }
 
   RObject classNames_robj = Rcpp::wrap(classNames); // convert + protect
-  return eval_call(r_func_py_filter_classes, (SEXP) classNames_robj);
+  RObject out = eval_call(r_func_py_filter_classes, (SEXP) classNames_robj);
+  return out;
+}
+
+
+SEXP py_class_names(PyObject* object) {
+  return py_class_names(object, (bool) PyExceptionInstance_Check(object));
 }
 
 
@@ -724,6 +730,27 @@ bool inherits2(SEXP object, const char* name) {
   return false;
 }
 
+bool inherits2(SEXP object, const char* name1, const char* name2) {
+  // like inherits in R, but iterates over the class STRSXP vector
+  // in reverse, since python.builtin.object is typically at the tail.
+  SEXP klass = Rf_getAttrib(object, R_ClassSymbol);
+  if (TYPEOF(klass) == STRSXP) {
+
+    int i = Rf_length(klass)-1;
+
+    for (; i >= 0; i--) {
+      if (strcmp(CHAR(STRING_ELT(klass, i)), name2) == 0) {
+        // found name2, now look for name1
+        for (i--; i >= 0; i--)
+          if (strcmp(CHAR(STRING_ELT(klass, i)), name1) == 0)
+            return true; // found name1 also
+        break; // did not find name1
+      }
+    }
+  }
+  return false;
+}
+
 //' Check if a Python object is a null externalptr
 //'
 //' @param x Python object
@@ -891,10 +918,10 @@ SEXP py_fetch_error(bool maybe_reuse_cached_r_trace) {
   // for shallow call stacks. So we fetch the call directly
   // using the R API.
   if (!PyObject_HasAttrString(excValue, "call")) {
-    // Technically we don't need to protct call, since
+    // Technically we don't need to protect call, since
     // it would already be protected by it's inclusion in the R callstack,
     // but rchk flags it anyway, and so ...
-    RObject r_call(  get_current_call() );
+    RObject r_call( get_current_call() );
     PyObject* r_call_capsule(py_capsule_new(r_call));
     PyObject_SetAttrString(excValue, "call", r_call_capsule);
     Py_DecRef(r_call_capsule);
@@ -943,22 +970,35 @@ class PyFlushOutputOnScopeExit {
 };
 
 
-// [[Rcpp::export]]
-std::string conditionMessage_from_py_exception(PyObjectRef exc) {
+
+std::string conditionMessage_from_py_exception(PyObject* exc) {
   // invoke 'traceback.format_exception_only(<traceback>)'
-  PyObjectPtr tb_module(py_import("traceback"));
-  if (tb_module.is_null())
-    return "<unknown python exception, traceback module not found>";
 
-  PyObjectPtr format_exception_only(
-      PyObject_GetAttrString(tb_module, "format_exception_only"));
-  if (format_exception_only.is_null())
-    return "<unknown python exception, traceback format fn not found>";
+  static PyObject* format_exception_only = []() {
+    PyObjectPtr tb_module(py_import("traceback"));
+    if (tb_module.is_null()) {
+      PyErr_Print();
+      Rcpp::stop("Failed to format Python Exception; could not import traceback module");
+    }
+
+    PyObject* format_exception_only = PyObject_GetAttrString(tb_module, "format_exception_only");
+
+    if (format_exception_only == NULL) {
+      PyErr_Print();
+      Rcpp::stop("Failed to format Python Exception; could not get traceback.format_exception_only");
+    }
+
+    return format_exception_only;
+  }();
+
 
   PyObjectPtr formatted(PyObject_CallFunctionObjArgs(
-      format_exception_only, Py_TYPE(exc.get()), exc.get(), NULL));
-  if (formatted.is_null())
-    return "<unknown python exception, traceback format fn returned NULL>";
+      format_exception_only, Py_TYPE(exc), exc, NULL));
+
+  if (formatted.is_null()) {
+    PyErr_Print();
+    Rcpp::stop("Failed to format Python Exception; traceback.format_exception_only() raised an Exception");
+  }
 
   // build error text
   std::ostringstream oss;
@@ -967,14 +1007,12 @@ std::string conditionMessage_from_py_exception(PyObjectRef exc) {
   for (Py_ssize_t i = 0, n = PyList_Size(formatted); i < n; i++)
     oss << as_std_string(PyList_GetItem(formatted, i));
 
-  static std::string hint;
-
-  if (hint.empty()) {
+  static std::string hint = []() {
     Environment pkg_env(Environment::namespace_env("reticulate"));
     Function hint_fn = pkg_env[".py_last_error_hint"];
     CharacterVector r_result = hint_fn();
-    hint = Rcpp::as<std::string>(r_result[0]);
-  }
+    return Rcpp::as<std::string>(r_result[0]);
+  }();
 
   oss << hint;
   std::string error = oss.str();
@@ -991,7 +1029,7 @@ std::string conditionMessage_from_py_exception(PyObjectRef exc) {
 
     std::string trunc("<...truncated...>");
 
-    // Tensorflow since ~2.6 has been including a currated traceback as part of
+    // TensorFlow since ~2.6 has been including a curated traceback as part of
     // the formatted exception message, with the most user-actionable content
     // towards the tail. Since the tail is the most useful part of the message,
     // truncate from the middle of the exception by default, after including the
@@ -1010,6 +1048,11 @@ std::string conditionMessage_from_py_exception(PyObjectRef exc) {
   return error;
 }
 
+// [[Rcpp::export]]
+std::string conditionMessage_from_py_exception(PyObjectRef exc) {
+  return conditionMessage_from_py_exception(exc.get());
+}
+
 // check whether the PyObject can be mapped to an R scalar type
 int r_scalar_type(PyObject* x) {
 
@@ -1223,6 +1266,8 @@ bool is_py_object(SEXP x) {
     case ENVSXP:
     case CLOSXP:
       return inherits2(x, "python.builtin.object");
+    case VECSXP:
+      return inherits2(x, "python.builtin.object", "condition");
     }
   }
   return false;
@@ -4375,3 +4420,27 @@ bool try_py_resolve_module_proxy(SEXP proxy) {
   Rcpp::Function py_resolve_module_proxy = pkgEnv["py_resolve_module_proxy"];
   return py_resolve_module_proxy(proxy);
 }
+
+
+
+SEXP py_exception_as_condition(PyObject* object, SEXP refenv) {
+  static SEXP names = []() {
+    SEXP names = Rf_allocVector(STRSXP, 2);
+    R_PreserveObject(names);
+    SET_STRING_ELT(names, 0, Rf_mkChar("message"));
+    SET_STRING_ELT(names, 1, Rf_mkChar("call"));
+    return names;
+  }();
+  SEXP out = PROTECT(Rf_allocVector(VECSXP, 2));
+
+  SET_VECTOR_ELT(out, 0, Rcpp::wrap(conditionMessage_from_py_exception(object)));
+  PyObject* call = py_get_attr(object, "call");
+  if(call != NULL)
+    SET_VECTOR_ELT(out, 1, py_to_r(call, true));
+
+  Rf_setAttrib(out, R_NamesSymbol, names);
+  Rf_setAttrib(out, R_ClassSymbol, Rf_getAttrib(refenv, R_ClassSymbol));
+  Rf_setAttrib(out, sym_py_object, refenv);
+  UNPROTECT(1);
+  return out;
+}

src/reticulate_types.h

@@ -11,11 +11,12 @@ using namespace reticulate::libpython;
 
 inline void python_object_finalize(SEXP object);
 SEXP py_callable_as_function(SEXP refenv, bool convert);
-SEXP py_class_names(PyObject*);
+SEXP py_class_names(PyObject*, bool exception);
 SEXP py_to_r_wrapper(SEXP ref);
 bool is_py_object(SEXP x);
 bool try_py_resolve_module_proxy(SEXP);
 SEXP new_refenv();
+SEXP py_exception_as_condition(PyObject*, SEXP refenv);
 
 extern SEXP sym_py_object;
 extern SEXP sym_convert;
@@ -41,19 +42,25 @@ class PyObjectRef: public Rcpp::RObject {
     SEXP xptr = PROTECT(R_MakeExternalPtr((void*) object, R_NilValue, R_NilValue));
     R_RegisterCFinalizer(xptr, python_object_finalize);
 
+
     SEXP refenv = PROTECT(new_refenv());
     Rf_defineVar(sym_pyobj, xptr, refenv);
     Rf_defineVar(sym_convert, Rf_ScalarLogical(convert), refenv);
     bool callable = PyCallable_Check(object);
-    if (callable || !simple)
+    bool exception = !callable && PyExceptionInstance_Check(object);
+    if (callable || exception || !simple)
       Rf_defineVar(sym_simple, Rf_ScalarLogical(false), refenv);
-    Rf_setAttrib(refenv, R_ClassSymbol, py_class_names(object));
+    Rf_setAttrib(refenv, R_ClassSymbol, py_class_names(object, exception));
 
     if (callable) {
       SEXP r_fn = PROTECT(py_callable_as_function(refenv, convert));
       r_fn = PROTECT(py_to_r_wrapper(r_fn));
       this->set__(r_fn); // PROTECT()
       UNPROTECT(4);
+    } else if (exception) {
+      SEXP r_cond = PROTECT(py_exception_as_condition(object, refenv));
+      this->set__(r_cond);
+      UNPROTECT(3);
     } else {
       this->set__(refenv);
       UNPROTECT(2);
@@ -97,11 +104,17 @@ class PyObjectRef: public Rcpp::RObject {
   SEXP get_refenv() const {
 
     SEXP sexp = this->get__();
-    while(TYPEOF(sexp) == CLOSXP)
+    unwrap:
+    switch(TYPEOF(sexp)) {
+    case ENVSXP:
+      break;
+    case CLOSXP:
+    case VECSXP:
       sexp = Rf_getAttrib(sexp, sym_py_object);
-
-    if (TYPEOF(sexp) != ENVSXP)
-      Rcpp::stop("malformed py_object, has type %s", Rf_type2char(TYPEOF(sexp)));
+      goto unwrap;
+    default:
+        Rcpp::stop("malformed py_object, has type %s", Rf_type2char(TYPEOF(sexp)));
+    }
 
     return sexp;
   }

tests/testthat/test-python-exceptions.R

@@ -139,3 +139,27 @@ def catch_clear_errstatus_then_raise_new_exception(fn):
 
 })
 
+
+test_that("confirm rlang/purrr can catch the exception", {
+  skip_if_no_python()
+
+  cnd <- tryCatch(
+    py_run_string("raise ZeroDivisionError"),
+    error = identity
+  )
+
+  # we already Suggests on rlang
+  expect_equal(rlang::cnd_type(cnd), "error")
+
+  # don't want to Suggests on purrr
+  requireNamespace_ <- requireNamespace
+  if (requireNamespace_("purrr", quietly = TRUE)) {
+    expect_error({
+      map <- yoink("purrr", "map")
+      map(1:3, function(i)
+        if (i == 2)
+          reticulate::py_run_string("raise Exception('fooobaar')"))
+    }, regexp = "In index: 2.*fooobaar")
+  }
+
+})