r-lib / later

Schedule an R function or formula to run after a specified period of time.
https://r-lib.github.io/later
Other
141 stars 28 forks source link

Error handling is no longer correct #191

Open jcheng5 opened 3 weeks ago

jcheng5 commented 3 weeks ago

Multiple tests in test-run_now.R are failing; we didn't catch this because they're disabled on ci.

In test-run_now.R, there are tests that test what happens if a callback:

  1. throws an error while run_now is called
  2. is interrupted while run_now is called

Tests in the second category apparently don't work on CRAN or CI environments. But tests in the first should be fine, and they've started failing since this commit in Rcpp (between 1.0.9 and 1.0.10, in 2022):

https://github.com/RcppCore/Rcpp/commit/d389a8a9540690fdc1839507abe95fdfdee8acd7

The problem is that our invoke_wrapped() logic assumes that Rcpp's unwind protect feature is not in use, and the default became to use it. With unwind protect, the right thing to do is almost to just call invoke() directly and not use invoke_wrapped() at all. I say almost, because this works great for errors but not for interrupts--the interrupts somehow aren't caught by tryCatch(..., interrupt = ...) so the tests abruptly end.