Dear cli maintainer, I recently encountered an issue ropensci/targets#1478 (please check this issue for more detailed examples)
The targets package probably tried using cli progress bar before initialized ("add") or after completion, resulting in an error. The related code is here
https://github.com/r-lib/cli/blob/main/R/progress-server.R#L356C1-L365C5
We see that shiny progress is set to NULL, and then trying to access functions such as output or set will raise errors.
bar$shiny_progress <- NULL
Basically if shiny ends the progress bar builtin_handler_shiny$complete() and the program still tries to call builtin_handler_shiny $output(), then the code will error out in shiny app. However, the code works fine outside of shiny, which makes the debugging process extremely hard.
I understand that if the package is used super carefully, then this issue will not occur. However, it is often the case that the package does not mandate what people do, especially when the code just works for most of people. Considering that other handlers do not raise errors on "use-after-free", and condition that triggers this error, this inconsistency is super hard to debug/find.
However, could we add checks just to make sure the functions are not that stringent? Alternatively, if you would like to ensure consistent behavior, we can force set and output to result in errors after completion for other functions.
builtin_handler_shiny <- list(
able = function(bar, .envir) {
"shiny" %in% loadedNamespaces() && asNamespace("shiny")$isRunning()
},
add = function(bar, .envir) {
bar$shiny_progress <- asNamespace("shiny")$Progress$new(
asNamespace("shiny")$getDefaultReactiveDomain(),
min = 0,
max = bar$total
)
bar$shiny_progress$set(
message = bar$name %||% "",
detail = shiny_detail(bar, .envir)
)
},
set = function(bar, .envir) {
if(!is.null(bar$shiny_progress)) {
bar$shiny_progress$set(
value = bar$current,
detail = shiny_detail(bar, .envir)
)
}
},
complete = function(bar, .envir, results) {
if (!is.null(bar$shiny_progress)) {
bar$shiny_progress$set(
value = bar$current,
detail = shiny_detail(bar, .envir)
)
bar$shiny_progress$close()
}
bar$shiny_progress <- NULL
},
output = function(bar, .envir, text) {
bar$shiny_output <-
last_lines(paste0(bar$shiny_output, " \u2022 ", text))
if(!is.null(bar$shiny_progress)) {
bar$shiny_progress$set(
value = bar$current,
detail = shiny_detail(bar, .envir)
)
}
}
)
Dear
climaintainer, I recently encountered an issue ropensci/targets#1478 (please check this issue for more detailed examples)The
targetspackage probably tried usingcliprogress bar before initialized ("add") or after completion, resulting in an error. The related code is herehttps://github.com/r-lib/cli/blob/main/R/progress-server.R#L356C1-L365C5
We see that shiny progress is set to
NULL, and then trying to access functions such asoutputorsetwill raise errors.Basically if shiny ends the progress bar
builtin_handler_shiny$complete()and the program still tries to callbuiltin_handler_shiny $output(), then the code will error out in shiny app. However, the code works fine outside of shiny, which makes the debugging process extremely hard.I understand that if the package is used super carefully, then this issue will not occur. However, it is often the case that the package does not mandate what people do, especially when the code just works for most of people. Considering that other handlers do not raise errors on "use-after-free", and condition that triggers this error, this inconsistency is super hard to debug/find.
However, could we add checks just to make sure the functions are not that stringent? Alternatively, if you would like to ensure consistent behavior, we can force
setandoutputto result in errors after completion for other functions.