Unwrap purrr's indexed errors (#306)

Author: DavisVaughan

NEWS.md

@@ -16,6 +16,9 @@
 * New `future_map_vec()`, `future_map2_vec()`, `future_pmap_vec()`, and
   `future_imap_vec()` to align with purrr (#261).
 
+* When a furrr function errors, purrr's error index is no longer confusingly
+  reported (#250).
+
 * furrr now looks up the purrr mapping function on the worker itself, rather
   than sending over its own copy of the function. This avoids possible issues
   when you have, say, purrr 1.0.0 locally but purrr 0.3.5 on the worker, where

R/template.R

@@ -438,7 +438,9 @@ furrr_template <- function(
     poll_progress(futures, file, n)
   }
 
-  values <- future::value(futures)
+  values <- furrr_try_catch({
+    future::value(futures)
+  })
 
   if (length(values) != length(chunks)) {
     abort("Internal error: Length of `values` not equal to length of `chunks`.")
@@ -460,6 +462,30 @@ furrr_template <- function(
 
 # ------------------------------------------------------------------------------
 
+furrr_try_catch <- function(expr) {
+  tryCatch(
+    expr = expr,
+    purrr_error_indexed = rethrow_purrr_error_indexed
+  )
+}
+
+rethrow_purrr_error_indexed <- function(cnd) {
+  # The purrr index information doesn't align with furrr's since the
+  # input is chunked along the workers. We rethrow the parent instead,
+  # which is the original error.
+  cnd <- cnd$parent
+
+  if (!is_condition(cnd)) {
+    # If something changes in purrr and `$parent` doesn't exist, just
+    # return and let normal erroring take its course
+    return()
+  }
+
+  rlang::cnd_signal(cnd)
+}
+
+# ------------------------------------------------------------------------------
+
 make_expr_seed_setup <- function(seed) {
   if (is.null(seed) || is_false(seed)) {
     return(NULL)

tests/testthat/_snaps/future-map.md

@@ -123,3 +123,43 @@
       Error in `future_map_vec()`:
       ! Can't combine `<output>[[1]]` <double> and `<output>[[2]]` <character>.
 
+# errors don't report purrr's indices (#250) / strategy - sequential / cores - 1
+
+    Code
+      future_map(x, fail_on_five)
+    Condition
+      Error in `...furrr_fn()`:
+      ! Failure!
+
+# errors don't report purrr's indices (#250) / strategy - multisession / cores - 1
+
+    Code
+      future_map(x, fail_on_five)
+    Condition
+      Error in `...furrr_fn()`:
+      ! Failure!
+
+# errors don't report purrr's indices (#250) / strategy - multisession / cores - 2
+
+    Code
+      future_map(x, fail_on_five)
+    Condition
+      Error in `...furrr_fn()`:
+      ! Failure!
+
+# errors don't report purrr's indices (#250) / strategy - multicore / cores - 1
+
+    Code
+      future_map(x, fail_on_five)
+    Condition
+      Error in `...furrr_fn()`:
+      ! Failure!
+
+# errors don't report purrr's indices (#250) / strategy - multicore / cores - 2
+
+    Code
+      future_map(x, fail_on_five)
+    Condition
+      Error in `...furrr_fn()`:
+      ! Failure!
+

tests/testthat/test-future-map.R

@@ -304,3 +304,18 @@ furrr_test_that("`...` globals/packages are found", {
     list(1, 1)
   )
 })
+
+furrr_test_that("errors don't report purrr's indices (#250)", {
+  fail_on_five <- function(x) {
+    if (x == 5) {
+      stop("Failure!")
+    }
+    1L
+  }
+
+  x <- 1:20
+
+  expect_snapshot(error = TRUE, {
+    future_map(x, fail_on_five)
+  })
+})