map() and pmap() remove class from error

[jennybc]

I noticed this when googledrive had a test failure, where the function in question uses a tryCatch():

https://github.com/tidyverse/googledrive/blob/7e8fa7abcb220566d2e1e99621b6e5d41e8f1c5b/R/shortcut.R#L184-L187

This resolve_one() function ultimately gets called inside pmap() and as of v1.0.0, the error no longer has the expected class.

---

With purrr 0.3.5, the class of an error thrown by .f is preserved. Here it's an oops_error.

f <- function(x) {
  if (x) {
    cli::cli_abort("oops", class = "oops_error")
  }
  x
}
f(FALSE)
#> [1] FALSE
f(TRUE)
#> Error in `f()`:
#> ! oops

#> Backtrace:
#>     ▆
#>  1. └─global f(TRUE)
#>  2.   └─cli::cli_abort("oops", class = "oops_error")
#>  3.     └─rlang::abort(...)

packageVersion("purrr")
#> [1] '0.3.5'

purrr::map(c(FALSE, TRUE), f)
#> Error in `.f()`:
#> ! oops

#> Backtrace:
#>     ▆
#>  1. └─purrr::map(c(FALSE, TRUE), f)
#>  2.   └─global .f(.x[[i]], ...)
#>  3.     └─cli::cli_abort("oops", class = "oops_error")
#>  4.       └─rlang::abort(...)
err <- rlang::catch_cnd(purrr::map(c(FALSE, TRUE), f))
err
#> <error/oops_error>
#> Error in `.f()`:
#> ! oops
#> ---
#> Backtrace:
#>  1. rlang::catch_cnd(purrr::map(c(FALSE, TRUE), f))
#>  8. purrr::map(c(FALSE, TRUE), f)
#>  9. global .f(.x[[i]], ...)
class(err)
#> [1] "oops_error"  "rlang_error" "error"       "condition"

---

But as of v1.0.0, the error no longer has that condition.

f <- function(x) {
  if (x) {
    cli::cli_abort("oops", class = "oops_error")
  }
  x
}
f(FALSE)
#> [1] FALSE
f(TRUE)
#> Error in `f()`:
#> ! oops

#> Backtrace:
#>     ▆
#>  1. └─global f(TRUE)
#>  2.   └─cli::cli_abort("oops", class = "oops_error")
#>  3.     └─rlang::abort(...)

packageVersion("purrr")
#> [1] '1.0.0'

purrr::map(c(FALSE, TRUE), f)
#> Error in `purrr::map()`:
#> ℹ In index: 2.
#> Caused by error in `.f()`:
#> ! oops

#> Backtrace:
#>     ▆
#>  1. └─purrr::map(c(FALSE, TRUE), f)
#>  2.   └─purrr:::map_("list", .x, .f, ..., .progress = .progress)
#>  3.     ├─purrr:::with_indexed_errors(...)
#>  4.     │ └─base::withCallingHandlers(...)
#>  5.     └─global .f(.x[[i]], ...)
#>  6.       └─cli::cli_abort("oops", class = "oops_error")
#>  7.         └─rlang::abort(...)
err <- rlang::catch_cnd(purrr::map(c(FALSE, TRUE), f))
err
#> <error/rlang_error>
#> Error in `purrr::map()`:
#> ℹ In index: 2.
#> Caused by error in `.f()`:
#> ! oops
#> ---
#> Backtrace:
#>   1. rlang::catch_cnd(purrr::map(c(FALSE, TRUE), f))
#>   8. purrr::map(c(FALSE, TRUE), f)
#>   9. purrr:::map_("list", .x, .f, ..., .progress = .progress)
#>  12. global .f(.x[[i]], ...)
class(err)
#> [1] "rlang_error" "error"       "condition"

[hadley]

Yeah, because it's wrapped inside another error now.

[jennybc]

Does that mean this is a behavioural change that's here to stay?

I understand the intent of this to be to re-throw the error, but with extra info about the offending element:

purrr/R/map.R

Line 205 in 54546b6

parent = cnd,

[lionel-]

I think we may need to implement in try_fetch() the same behaviour as in expect_error(), i.e. matching across the parent ancestry of the condition.

Currently you need to implement this behaviour manually using cnd_inherits():

foo <- function() abort("foo", class = "foo")

# Using `try_fetch()` to easily return a value or ignore the condition
try_fetch(
  purrr::map(1, \(x) foo()),
  # Inspect all errors
  error = function(cnd) {
    if (cnd_inherits(cnd, "foo")) {
      # We got a match, return a value
      class(cnd)
    } else {
      # Ignore the ones that don't match
      zap()
    }
  }
)
#> [1] "rlang_error" "error"       "condition"

If try_fetch() supported this natively, this would be:

try_fetch(
  purrr::map(1, \(x) foo()),
  foo = function(cnd) class(cnd)
)

[lionel-]

oops, the complicated try_fetch() call is not even right. Using cnd_inherits() to find out if there is a match is not sufficient, because we still have the child error in cnd and we want the parent error. Here is a working version:

try_fetch(
  purrr::map(1, \(x) foo()),
  error = function(cnd) {
    repeat {
      if (is_null(cnd)) {
        return(zap())
      }
      if (inherits(cnd, "foo")) {
        return(class(cnd))
      }
      cnd <- cnd[["parent"]]
    }
  }
)
#> [1] "foo"         "rlang_error" "error"       "condition"

[hadley]

We’re likely to provide a way out of the error wrapping in a patch release in early Jan.

[lionel-]

Just to be clear, disabling the error wrapping only helps when you don't want the error wrapping, i.e. no index iteration info in error messages. It'd still be helpful to support parent errors in try_fetch() for the other cases. Also it's not possible to disable error wrapping in full generality, for instance in dplyr verbs.

[mlane3]

Just to be clear, disabling the error wrapping only helps when you don't want the error wrapping, i.e. no index iteration info in error messages. It'd still be helpful to support parent errors in try_fetch() for the other cases. Also it's not possible to disable error wrapping in full generality, for instance in dplyr verbs.

I just mentioned this because lionel, point error wrapping being needed is really important.

[gadenbuie]

I think it'd be helpful to call this out in the NEWS as a breaking change. I think it'd also be worth calling out rlang::cnd_inherits() as a workaround, noting that the underlying error is now the parent of the thrown error. In my experience so far, I mostly need to move classed error handling via tryCatch() into the error handler to use rlang::cnd_inherits() instead.

Code from before purrr 1.0.0

abort_special <- function(x) rlang::abort("my error", class = "error_special")

## purrr <= 0.3.5 ----
tryCatch(
  purrr::map(1, abort_special),
  error_special = function(cnd) {
    message("handling special error...")
    cnd
  }
)
#> handling special error...
#> <error/error_special>
#> Error in `.f()`:
#> ! my error
#> ---
#> Backtrace:
#>  1. base::tryCatch(...)
#>  5. purrr::map(1, abort_special)
#>  6. global .f(.x[[i]], ...)

now becomes the following

## purrr >= 1.0.0 ----
tryCatch(
  purrr::map(1, abort_special),
  error = function(cnd) {
    if (rlang::cnd_inherits(cnd, "error_special")) {
      message("handling special error...")
      cnd$parent
    } else {
      stop(cnd)
    }
  }
)
#> handling special error...
#> <error/error_special>
#> Error in `.f()`:
#> ! my error
#> ---
#> Backtrace:
#>  1. base::tryCatch(...)
#>  5. purrr::map(1, abort_special)
#>  6. purrr:::map_("list", .x, .f, ..., .progress = .progress)
#>  9. global .f(.x[[i]], ...)

If the message handler doesn't need to access data in the underlying error (i.e. taking out cnd and cnd$parent from the handlers above), both methods are equivalent and the second pattern will also work for purrr < 1.0.0.

Edited: the non-special condition has to be rethrown or errors are silently swallowed; thanks for the reminder @hadley.

[hadley]

Sounds like this issue needs three things:

1. An update to the NEWS to clearly indicate this breaking change
2. A way to opt-out (Find a solution for the inverted chained error problem #1022)
3. Some documentation about how to work around if you want to keep the iteration number (i.e. probably a custom error class and some docs around using cnd_inherits)

[hadley]

@gadenbuie there's a subtle error in your code. I think you want:

tryCatch(
  purrr::map(1, abort_special),
  error = function(cnd) {
    if (rlang::cnd_inherits(cnd$parent, "error_special")) {
      message("handling special error...")
      cnd$parent
    } else {
     stop(cnd)
    }
  }
)

Otherwise other errors will be silently swallowed.

[lionel-]

@gadenbuie Also tryCatch() has big downsides for rethrowing errors. It prevents recover() and backtrace capture from reaching in the nested context. For this reason it's always preferable to use withCallingHandlers() or try_fetch() for the use case of rethrowing errors.

[hadley]

@mlane3 please don't @ mention people in your replies. You have failed to clearly explain what your problem is and your large number of comments on the purrr repo are not helping. As an open source team, we only have a limited amount of time to focus on problems, like this, that only appear to affect a very small number of individuals.