Skip to content

Commit eac0eea

Browse files
authored
fix: Wrap extended task invocation in promise_resolve() (#4225)
* fix: Wrap extended task invocation in `promise_resolve()` * refactor: cleanup error handling and promise chain * chore: add news entry
1 parent 6df0bb9 commit eac0eea

File tree

2 files changed

+30
-28
lines changed

2 files changed

+30
-28
lines changed

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
* Shiny's Typescript assets are now compiled to ES2021 instead of ES5. (#4066)
2525

26+
* `ExtendedTask` now catches synchronous values and errors and returns them via `$result()`. Previously, the extended task function was required to always return a promise. This change makes it easier to use `ExtendedTask` with a function that may return early or do some synchronous work before returning a promise. (#4225)
27+
2628
## Bug fixes
2729

2830
* Fixed a bug with modals where calling `removeModal()` too quickly after `showModal()` would fail to remove the modal if the remove modal message was received while the modal was in the process of being revealed. (#4173)
@@ -31,6 +33,8 @@
3133

3234
* Updated the JavaScript used when inserting a tab to avoid rendering dynamic UI elements twice when adding the new tab via `insertTab()` or `bslib::nav_insert()`. (#4179)
3335

36+
* Fixed an issue with `ExtendedTask` where synchronous errors would cause an error that would stop the current session. (#4225)
37+
3438
# shiny 1.10.0
3539

3640
## New features and improvements

R/extended-task.R

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,15 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE,
130130
#' arguments.
131131
invoke = function(...) {
132132
args <- rlang::dots_list(..., .ignore_empty = "none")
133+
call <- rlang::caller_call(n = 0)
133134

134135
if (
135136
isolate(private$rv_status()) == "running" ||
136137
private$invocation_queue$size() > 0
137138
) {
138-
private$invocation_queue$add(args)
139+
private$invocation_queue$add(list(args = args, call = call))
139140
} else {
140-
private$do_invoke(args)
141+
private$do_invoke(args, call = call)
141142
}
142143
invisible(NULL)
143144
},
@@ -204,44 +205,41 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE,
204205
rv_error = NULL,
205206
invocation_queue = NULL,
206207

207-
do_invoke = function(args) {
208+
do_invoke = function(args, call = NULL) {
208209
private$rv_status("running")
209210
private$rv_value(NULL)
210211
private$rv_error(NULL)
211212

212-
p <- NULL
213-
tryCatch({
214-
maskReactiveContext({
215-
# TODO: Bounce the do.call off of a promise_resolve(), so that the
216-
# call to invoke() always returns immediately?
217-
result <- do.call(private$func, args)
218-
p <- promises::as.promise(result)
219-
})
220-
}, error = function(e) {
221-
private$on_error(e)
222-
})
213+
p <- promises::promise_resolve(
214+
maskReactiveContext(do.call(private$func, args))
215+
)
223216

224-
promises::finally(
225-
promises::then(p,
226-
onFulfilled = function(value, .visible) {
227-
private$on_success(list(value=value, visible=.visible))
228-
},
229-
onRejected = function(error) {
230-
private$on_error(error)
231-
}
232-
),
233-
onFinally = function() {
234-
if (private$invocation_queue$size() > 0) {
235-
private$do_invoke(private$invocation_queue$remove())
236-
}
217+
p <- promises::then(
218+
p,
219+
onFulfilled = function(value, .visible) {
220+
private$on_success(list(value = value, visible = .visible))
221+
},
222+
onRejected = function(error) {
223+
private$on_error(error, call = call)
237224
}
238225
)
239226

227+
promises::finally(p, onFinally = function() {
228+
if (private$invocation_queue$size() > 0) {
229+
next_call <- private$invocation_queue$remove()
230+
private$do_invoke(next_call$args, next_call$call)
231+
}
232+
})
240233

241234
invisible(NULL)
242235
},
243236

244-
on_error = function(err) {
237+
on_error = function(err, call = NULL) {
238+
cli::cli_warn(
239+
"ERROR: An error occurred when invoking the ExtendedTask.",
240+
parent = err,
241+
call = call
242+
)
245243
private$rv_status("error")
246244
private$rv_error(err)
247245
},

0 commit comments

Comments
 (0)