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.

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

Fix run now errors and interrupts #192

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jcheng5
Copy link
Member

@jcheng5 jcheng5 commented Oct 25, 2024

Fixes #191

The introduction of unwind protection in Rcpp required us to stop
working so hard to catch and re-propagate different kinds of errors
across the R->C++->R boundary. This went unnoticed for so long
because the interrupt-related tests were already misbehaving on
CI and on CRAN. I've now refactored the tests so that the test
skipping can be narrowly scoped to just the interrupts, and even
those, I'd like to try again on CI.
@jcheng5 jcheng5 force-pushed the fix-run-now-errors-and-interrupts branch from f873492 to 2922839 Compare October 25, 2024 05:21
@jcheng5
Copy link
Member Author

jcheng5 commented Oct 25, 2024

Re: remaining failures for R 4.1 and 3.6 on Windows: On these builds, having Rcpp code throw an exception that's caught by the calling R code, seems to be escaping the try/catch somehow. But only for i386, not x64!?

@jcheng5
Copy link
Member Author

jcheng5 commented Oct 25, 2024

Minimal repro looks like the below. It actually works when called from RStudio, but fails when called from R at a terminal or if you put it in a file and call R -f test.R.

library(later)

Rcpp::cppFunction(
  depends = "later",
  includes = '
      #include <later_api.h>

      void oof(void* data) {
        throw std::runtime_error("This is a C++ exception.");
      }
    ',
  code = '
      void cpp_error(int value) {
        later::later(oof, 0, 0);
      }
    '
)

tryCatch(
  {cpp_error(1); later::run_now()},
  error = function(e) {message("got here")}
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error handling is no longer correct
1 participant