HenrikBengtsson / future

:rocket: R package: future: Unified Parallel and Distributed Processing in R for Everyone
https://future.futureverse.org
946 stars 82 forks source link

future(message(<error>)) circumvents the muffling of the error resignaled #507

Closed HenrikBengtsson closed 2 years ago

HenrikBengtsson commented 3 years ago

From https://stackoverflow.com/questions/66083734/handling-exceptions-using-trycatch-inside-future-applyfuture-lapply/67815186#67815186:

Issue

Calling message(cond) on a captured error condition cond are caught by the future despite being muffled internally by message(). Same for warning().

boom <- function(x) {
  tryCatch(stop("boom"), error = function(e) {
    message(1)
    message(e)
    message(2)
  })
}  

y <- lapply(1L, FUN = boom)
## 1
## boom2

y <- future.apply::future_lapply(1L, FUN = boom)
## 1
## Error in doTryCatch(return(expr), name, parentenv, handler) : boom

Troubleshooting

Both message() and warning() use something like:

    withRestarts({
        signalCondition(cond)
        cat(conditionMessage(cond), file = stderr())
    }, muffleMessage = function() NULL)

internally. Indeed, if we do:

boom <- function(x) {
  tryCatch(stop("boom"), error = function(cond) {
    message(1)
    withRestarts({
        signalCondition(cond)
        cat(conditionMessage(cond), file = stderr())
    }, muffleMessage = function() NULL)
    message(2)
  })
}

y <- lapply(1L, FUN = boom)
## 1
## boom2

y <- future.apply::future_lapply(1L, FUN = boom)
## 1
## Error in doTryCatch(return(expr), name, parentenv, handler) : boom

More details:

library(future)
f <- future(boom(1))
r <- result(f)
str(r)
## List of 6
##  $ value     : NULL
##  $ visible   : NULL
##  $ conditions:List of 2
##   ..$ :List of 2
##   .. ..$ condition:List of 2
##   .. .. ..$ message: chr "1\n"
##   .. .. ..$ call   : language message(1)
##   .. .. .. ..- attr(*, "srcref")= 'srcref' int [1:8] 3 5 3 14 5 14 3 3
##   .. .. .. .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x557f560e1328> 
##   .. .. ..- attr(*, "class")= chr [1:3] "simpleMessage" "message" "condition"
##   .. ..$ signaled : int 0
##   ..$ :List of 5
##   .. ..$ condition:List of 2
##   .. .. ..$ message: chr "boom"
##   .. .. ..$ call   : language doTryCatch(return(expr), name, parentenv, handler)
##   .. .. ..- attr(*, "class")= chr [1:3] "simpleError" "error" "condition"
##   .. ..$ calls    :List of 10
##   .. .. ..$ : language eval(quote(boom(1)), new.env())
##   .. .. ..$ : language boom(1)
##   .. .. ..$ : language tryCatch(stop("boom"), error = function(cond) {     message(1) ...
##   .. .. .. ..- attr(*, "srcref")= 'srcref' int [1:8] 2 3 11 4 3 4 2 11
##   .. .. .. .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x557f560e1328> 
##   .. .. ..$ : language tryCatchList(expr, classes, parentenv, handlers)
##   .. .. ..$ : language tryCatchOne(expr, names, parentenv, handlers[[1L]])
##   .. .. ..$ : language value[[3L]](cond)
##   .. .. ..$ : language withRestarts({     signalCondition(cond) ...
##   .. .. .. ..- attr(*, "srcref")= 'srcref' int [1:8] 4 5 9 6 5 6 4 9
##   .. .. .. .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x557f560e1328> 
##   .. .. ..$ : language withOneRestart(expr, restarts[[1L]])
##   .. .. ..$ : language doWithOneRestart(return(expr), restart)
##   .. .. ..$ : language doTryCatch(return(expr), name, parentenv, handler)
##   .. ..$ session  :List of 6
##   .. .. ..$ r         :List of 14
##   .. .. .. ..$ platform      : chr "x86_64-pc-linux-gnu"
##   .. .. .. ..$ arch          : chr "x86_64"
##   .. .. .. ..$ os            : chr "linux-gnu"
##   .. .. .. ..$ system        : chr "x86_64, linux-gnu"
##   .. .. .. ..$ status        : chr "Patched"
##   .. .. .. ..$ major         : chr "4"
##   .. .. .. ..$ minor         : chr "1.0"
##   .. .. .. ..$ year          : chr "2021"
##   .. .. .. ..$ month         : chr "05"
##   .. .. .. ..$ day           : chr "18"
##   .. .. .. ..$ svn rev       : chr "80320"
##   .. .. .. ..$ language      : chr "R"
##   .. .. .. ..$ version.string: chr "R version 4.1.0 Patched (2021-05-18 r80320)"
##   .. .. .. ..$ nickname      : chr "Camp Pontanezen"
##   .. .. ..$ locale    : chr "LC_CTYPE=en_US.UTF-8;LC_NUMERIC=C;LC_TIME=en_US.UTF-8;LC_COLLATE=en_US.UTF-8;LC_MONETARY=en_US.UTF-8;LC_MESSAGE"| __truncated__
##   .. .. ..$ rngkind   : chr [1:3] "Mersenne-Twister" "Inversion" "Rejection"
##   .. .. ..$ namespaces: chr [1:36] "progressr" "codetools" "grDevices" "listenv" ...
##   .. .. ..$ search    : chr [1:13] ".GlobalEnv" "package:future" "package:stats" "package:graphics" ...
##   .. .. ..$ system    : Named chr [1:8] "Linux" "5.4.0-73-generic" "#82~18.04.1-Ubuntu SMP Fri Apr 16 15:10:02 UTC 2021" "hb-x1" ...
##   .. .. .. ..- attr(*, "names")= chr [1:8] "sysname" "release" "version" "nodename" ...
##   .. ..$ timestamp: POSIXct[1:1], format: "2021-06-02 21:33:06"
##   .. ..$ signaled : int 0
##  $ rng       : logi FALSE
##  $ version   : chr "1.8"
##  $ stdout    : chr ""
##  - attr(*, "class")= chr "FutureResult"
HenrikBengtsson commented 2 years ago

Just so I don't forget: re-reading my comment on StackOverflow made me remember that one possibly workaround might be to inspect the call stack to detect the special cases of message(<error>) and warning(<error>). It could be that we can do that from within the future.

The cost for such extra overhead would only apply to error conditions, and since they are often rare, it would not be a dramatic price to pay.

HenrikBengtsson commented 2 years ago

After further investigation, I consider this a design bug in both message() and warning(). I've posted this to R-devel thread 'message() and warning() circumvent calling handlers and signal the original class, e.g. an error' on 2022-03-01 (https://stat.ethz.ch/pipermail/r-devel/2022-March/081515.html). Let's see what the verdict will be.

/cc @DavisVaughan

HenrikBengtsson commented 2 years ago

Luke T replied and said that the current behavior is expected and by design. From his reply, I conclude that using message(e) as in:

tryCatch(stop("boom"), error = function(e) message(e))

is incorrect and that one should indeed use:

tryCatch(stop("boom"), error = function(e) message(conditionMessage(e)))
HenrikBengtsson commented 2 years ago

Documented, cf. https://future.futureverse.org/articles/future-4-issues.html#capturing-errors-outputting-their-messages-and-returning-a-default-value