Re-raise R callback errors by value, not via std::exception_ptr (#3, part 1/2)

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: RcppTrust
 Type: Package
 Title: Thread-Safe C++ Trust Region Optimizers
-Version: 0.2.0
+Version: 0.2.1
 Authors@R: c(
     person(given = c("Charles", "J."), family = "Geyer", role = c("aut", "cph"),
         email = "geyer@umn.edu", comment = c(ORCID = "0000-0003-1471-1703")),

NEWS.md

@@ -1,3 +1,11 @@
+# RcppTrust 0.2.1
+
+* Errors raised inside the R callbacks of `bobyqa()`, `newuoa()` and
+  `steihaug()` are again reported unchanged on clang/libc++ builds (CRAN's
+  fedora-clang check showed "c++ exception (unknown reason)"). The error
+  is now stored by value and re-thrown afresh instead of going through
+  `std::exception_ptr` (#3).
+
 # RcppTrust 0.2.0
 
 * The package license is now GPL-2, because the BOBYQA/NEWUOA ports and

src/minqa_r_interface.cpp

@@ -4,15 +4,15 @@
 // *_solve_c() cores a C/C++ consumer calls -- only the objective callback
 // (which evaluates an R closure, so is main-thread only) differs.
 #include <Rcpp.h>
-#include <exception>
+#include "r_callback_error.h"
 #include "minqa_types.h"
 using namespace Rcpp;
 
 namespace {
 
 struct MinqaRObjfun {
   SEXP fn;
-  std::exception_ptr error;
+  RCallbackError error;
 };
 
 // Mirrors minqa's calfun(): Rf_asReal(fn(x)), evaluated in fn's
@@ -31,7 +31,7 @@ int minqa_r_objfun(int n, const double *x, double *f, void *userdata) {
     *f = Rf_asReal(v);
     return 0;
   } catch (...) {
-    d->error = std::current_exception();
+    d->error.capture();
     return -1;
   }
 }
@@ -53,9 +53,9 @@ minqa_options_t minqa_r_options(Environment ctrl) {
 // Shape of minqa's rval() list (before the R shim adds `msg`).
 List minqa_r_result(int n, minqa_result_t &res, MinqaRObjfun &d,
                     const char *cls) {
-  if (d.error) {
+  if (d.error.failed()) {
     minqa_result_free(&res);
-    std::rethrow_exception(d.error);
+    d.error.rethrow();
   }
   if (res.ierr == MINQA_ERR_NONFINITE_X) {
     minqa_result_free(&res);
@@ -83,7 +83,7 @@ List minqa_r_result(int n, minqa_result_t &res, MinqaRObjfun &d,
 List minqa_bobyqa_r(NumericVector par, NumericVector lower, NumericVector upper,
                     Environment ctrl, Function fn) {
   int n = par.size();
-  MinqaRObjfun d{fn, nullptr};
+  MinqaRObjfun d{fn, {}};
   minqa_options_t opts = minqa_r_options(ctrl);
   minqa_result_t res;
   bobyqa_solve_c(n, par.begin(), lower.begin(), upper.begin(), minqa_r_objfun,
@@ -94,7 +94,7 @@ List minqa_bobyqa_r(NumericVector par, NumericVector lower, NumericVector upper,
 // [[Rcpp::export]]
 List minqa_newuoa_r(NumericVector par, Environment ctrl, Function fn) {
   int n = par.size();
-  MinqaRObjfun d{fn, nullptr};
+  MinqaRObjfun d{fn, {}};
   minqa_options_t opts = minqa_r_options(ctrl);
   minqa_result_t res;
   newuoa_solve_c(n, par.begin(), minqa_r_objfun, &d, &opts, &res);

src/r_callback_error.h

@@ -0,0 +1,57 @@
+#ifndef RCPPTRUST_R_CALLBACK_ERROR_H
+#define RCPPTRUST_R_CALLBACK_ERROR_H
+
+// Captures an error raised inside an R callback so it can be re-raised
+// once the thread-safe solver core has unwound. The error is stored by
+// value and re-thrown as a fresh exception rather than through
+// std::exception_ptr/std::rethrow_exception: with clang + libc++ (CRAN's
+// fedora-clang) the rethrown object no longer matched Rcpp's catch
+// clauses and surfaced as "c++ exception (unknown reason)".
+#include <Rcpp.h>
+#include <exception>
+#include <string>
+
+class RCallbackError {
+ public:
+  // Call only from inside a catch block.
+  void capture() {
+    try {
+      throw;
+    } catch (Rcpp::internal::InterruptedException &) {
+      kind_ = INTERRUPT;
+    } catch (Rcpp::LongjumpException &e) {
+      // The unwind token resumes the original R condition unchanged.
+      kind_ = LONGJUMP;
+      token_ = e.token;
+    } catch (std::exception &e) {
+      kind_ = MESSAGE;
+      msg_ = e.what();
+    } catch (...) {
+      kind_ = MESSAGE;
+      msg_ = "unknown C++ exception in R callback";
+    }
+  }
+
+  bool failed() const { return kind_ != NONE; }
+
+  // Re-raise the captured error (no-op if nothing was captured). Under
+  // Rcpp's unwind protection an R-level interrupt arrives as a
+  // LongjumpException, and callers only rethrow after failed(), so the
+  // INTERRUPT and NONE cases are defensive and not reachable from tests.
+  void rethrow() const {
+    switch (kind_) {
+      case INTERRUPT: throw Rcpp::internal::InterruptedException();  // # nocov
+      case LONGJUMP: throw Rcpp::LongjumpException(token_);
+      case MESSAGE: throw Rcpp::exception(msg_.c_str(), false);
+      case NONE: break;  // # nocov start
+    }
+  }  // # nocov end
+
+ private:
+  enum Kind { NONE, INTERRUPT, LONGJUMP, MESSAGE };
+  Kind kind_ = NONE;
+  Rcpp::RObject token_;  // keeps the unwind token protected
+  std::string msg_;
+};
+
+#endif

src/steihaug_r_interface.cpp

@@ -3,16 +3,16 @@
 // C/C++ consumer calls; only the callbacks (which evaluate R closures, so
 // are main-thread only) differ.
 #include <Rcpp.h>
-#include <exception>
 #include <string>
+#include "r_callback_error.h"
 #include "steihaug_types.h"
 using namespace Rcpp;
 
 namespace {
 
 struct SteihaugRObjfun {
   SEXP fn, gr, hess, hessvec;  // hess/hessvec may be R_NilValue
-  std::exception_ptr error;
+  RCallbackError error;
 };
 
 void steihaug_r_copy(SEXP v, int len, double *out, const char *what) {
@@ -50,7 +50,7 @@ int steihaug_r_objfun(int n, const double *x, double *value, double *gradient,
     }
     return 0;
   } catch (...) {
-    d->error = std::current_exception();
+    d->error.capture();
     return -1;
   }
 }
@@ -64,7 +64,7 @@ int steihaug_r_hessvec(int n, const double *x, const double *v, double *hv,
     steihaug_r_copy(hvf(xx, vv), n, hv, "hessvec");
     return 0;
   } catch (...) {
-    d->error = std::current_exception();
+    d->error.capture();
     return -1;
   }
 }
@@ -101,7 +101,7 @@ double ctrl_tol(List ctrl, const char *nm) {
 List steihaug_solve_r(NumericVector par, Function fn, Function gr,
                       RObject hess, RObject hessvec, List control) {
   int n = par.size();
-  SteihaugRObjfun d{fn, gr, hess, hessvec, nullptr};
+  SteihaugRObjfun d{fn, gr, hess, hessvec, {}};
 
   steihaug_options_t o = steihaug_options_default();
   o.initial_radius = ctrl_dbl(control, "rinit");
@@ -132,9 +132,9 @@ List steihaug_solve_r(NumericVector par, Function fn, Function gr,
                    hessvec.isNULL() ? nullptr : steihaug_r_hessvec, &d, &o,
                    &res);
 
-  if (d.error) {
+  if (d.error.failed()) {
     steihaug_result_free(&res);
-    std::rethrow_exception(d.error);
+    d.error.rethrow();
   }
   if (res.status == STEIHAUG_ERR_INVALID_OPTIONS) {
     steihaug_result_free(&res);

tests/testthat/test-minqa.R

@@ -123,6 +123,19 @@ test_that("R errors inside fn propagate unchanged", {
   expect_error(bobyqa(c(1, 2), bad, lower = 0, upper = 4), "boom")
   expect_error(bobyqa(c(5, 2), fr, lower = 0, upper = 4),
                "Starting values violate bounds")
+  # The original condition object (and its class) reaches the caller.
+  raise <- function(x) {
+    stop(structure(
+      class = c("myErr", "error", "condition"),
+      list(message = "custom", call = NULL)
+    ))
+  }
+  expect_error(newuoa(c(1, 2), raise), "custom", class = "myErr")
+  expect_error(
+    bobyqa(c(1, 2), raise, lower = 0, upper = 4),
+    "custom",
+    class = "myErr"
+  )
 })
 
 test_that("extra arguments and names are passed to fn", {

tests/testthat/test-steihaug.R

@@ -55,6 +55,18 @@ test_that("errors and malformed returns in R callbacks propagate", {
                "gradfail")
   expect_error(steihaug(c(-1.2, 1), fr, function(x) 1, hr), "length 2")
   expect_error(steihaug(c(-1.2, 1), fr, grr, function(x) diag(3)), "2 by 2")
+  raise <- function(...) {
+    stop(structure(
+      class = c("myErr", "error", "condition"),
+      list(message = "custom", call = NULL)
+    ))
+  }
+  expect_error(steihaug(c(-1.2, 1), raise, grr, hr), "custom", class = "myErr")
+  expect_error(
+    steihaug(c(-1.2, 1), fr, grr, hessvec = raise),
+    "custom",
+    class = "myErr"
+  )
 })
 
 test_that("infinite trial values are rejected, not accepted", {