diff --git a/R/domains.R b/R/domains.R index 94972499..cf5ecf1c 100644 --- a/R/domains.R +++ b/R/domains.R @@ -108,30 +108,35 @@ wrap_callback_reenter <- function(callback, domain) { force(callback) force(domain) - wrapper <- function(...) { - # replace = TRUE because we don't care what the current domain is; we're - # (temporarily) putting the world back to the way it was when the callback - # was bound to a promise. - reenter_promise_domain(domain, callback(...), replace = TRUE) + # We can't simply take `...` as arguments and call `callback(...)`. There are + # parts of this package that will inspect formals() to see if there's a + # `.visible` parameter in the callback. Using `match.call()` here ensures that + # the callback is called with the same arguments as it would have been if it + # were called directly. + # + # IMPORTANT NOTE: This technique changes callback arguments from lazy eval to + # eager eval--make sure you're OK with that before using! + wrapper <- function() { + # Evaluate the arguments in the caller's environment + call <- match.call() + # The `[-1]` is to drop the function name from the call--we just want the + # arguments + args <- lapply(call[-1], eval, parent.frame()) + # Create and evaluate the callback call in our environment, wrapped in reenter_promise_domain + reenter_promise_domain( + domain, + rlang::exec(callback, !!!args), + # replace = TRUE because we don't care what the current domain is; we're + # (temporarily) putting the world back to the way it was when the callback + # was bound to a promise. + replace = TRUE + ) } + # Copy the formals from the original callback to the wrapper, so that the + # wrapper can be called with the same arguments as the original callback. + formals(wrapper) <- formals(callback) - # There are parts of this package that will inspect formals() to see if - # there's a `.visible` parameter in the callback. So it's important to have - # the returned wrapper have the same formals as the original callback. - wrap_with_signature(wrapper, formals(callback)) -} - -wrap_with_signature <- function(func, formal_args) { - # func must have a `...` signature - stopifnot("..." %in% names(formals(func))) - - args <- names(formal_args) - recall <- rlang::call2( - func, - !!!rlang::set_names(lapply(args, as.symbol), args) - ) - - rlang::new_function(formal_args, recall) + wrapper } globals <- new.env(parent = emptyenv()) diff --git a/R/promise.R b/R/promise.R index 9fba2e14..b2bcae55 100644 --- a/R/promise.R +++ b/R/promise.R @@ -3,110 +3,244 @@ #' @import later NULL +PromiseStateResolved <- R6::R6Class( + "PromiseStateResolved", + private = list( + value = NULL, + visible = TRUE + ), + public = list( + initialize = function(value, visible) { + private$value <- value + private$visible <- visible + }, + status = function() { + "fulfilled" + }, + format = function() { + classname <- class(private$value)[[1]] + if (length(classname) == 0) classname <- "" + sprintf("", self$status(), classname) + }, + register = function(onFulfilled = NULL, onRejected = NULL) { + force(onFulfilled) + force(onRejected) + + if (!is.null(onFulfilled)) { + later::later(function() { + lapply(onFulfilled, function(f) { + f(private$value, private$visible) + }) + }) + } + } + ) +) + +PromiseStateRejected <- R6::R6Class( + "PromiseStateRejected", + private = list( + reason = NULL + ), + public = list( + initialize = function(reason) { + private$reason <- reason + }, + status = function() { + "rejected" + }, + format = function() { + classname <- class(private$reason)[[1]] + if (length(classname) == 0) classname <- "" + sprintf("", self$status(), classname) + }, + register = function(onFulfilled = NULL, onRejected = NULL) { + force(onFulfilled) + force(onRejected) + + if (!is.null(onRejected)) { + later::later(function() { + lapply(onRejected, function(f) { + f(private$reason) + }) + }) + } + } + ) +) + +PromiseStatePending <- R6::R6Class( + "PromiseStatePending", + private = list( + onFulfilled = list(), + onRejected = list() + ), + public = list( + initialize = function() { + private$onFulfilled <- list() + private$onRejected <- list() + }, + status = function() { + "pending" + }, + format = function() { + "" + }, + register = function(onFulfilled = NULL, onRejected = NULL) { + force(onFulfilled) + force(onRejected) + + if (!is.null(onFulfilled)) { + stopifnot(is.list(onFulfilled)) + private$onFulfilled <- c(private$onFulfilled, onFulfilled) + } + if (!is.null(onRejected)) { + stopifnot(is.list(onRejected)) + private$onRejected <- c(private$onRejected, onRejected) + } + invisible(NULL) + }, + resolve = function(value, visible) { + force(value) + force(visible) + + # Resolving to promise should've been handled at a higher level + stopifnot(!is.promising(value)) + + new_state <- PromiseStateResolved$new(value, visible) + new_state$register(private$onFulfilled, private$onRejected) + private$onFulfilled <- list() + private$onRejected <- list() + new_state + }, + reject = function(reason) { + force(reason) + + new_state <- PromiseStateRejected$new(reason) + new_state$register(private$onFulfilled, private$onRejected) + private$onFulfilled <- list() + private$onRejected <- list() + new_state + }, + chain = function(new_state) { + force(new_state) + + if (identical(self, new_state)) { + new_new_state <- self$reject(simpleError( + "Chaining cycle detected for promise" + )) + return(new_new_state) + } + + new_state$register(private$onFulfilled, private$onRejected) + private$onFulfilled <- list() + private$onRejected <- list() + new_state + } + ) +) + + +PromiseStatePointer <- R6::R6Class( + "PromiseStatePointer", + public = list( + state = NULL, + initialize = function(state) { + self$state <- state + } + ) +) + #' @import R6 Promise <- R6::R6Class( "Promise", private = list( - state = "pending", - value = NULL, - visible = TRUE, + ppstate = NULL, publicResolveRejectCalled = FALSE, - onFulfilled = list(), - onRejected = list(), - onFinally = list(), - rejectionHandled = FALSE, - - # Private resolve/reject differs from public resolve/reject - # in that the private versions are allowed to be called - # more than once, whereas public ones no-op after the first - # time they are invoked. + # If FALSE, then either then() has been called on this specific object, or + # this object has been used to resolve() another promise. We use this to + # determine whether to print a warning if rejection is not handled. + isTerminal = TRUE, + + pstate = function() { + private$ppstate$state + }, + doResolve = function(value) { val <- withVisible(value) value <- val$value visible <- val$visible + stopifnot(private$pstate()$status() == "pending") + if (is.promising(value)) { - value <- as.promise(value) - if (identical(self, attr(value, "promise_impl", exact = TRUE))) { - return(private$doReject(simpleError( - "Chaining cycle detected for promise" - ))) - } - # This then() call doesn't need promise domains; semantically, it doesn't - # really exist, as it's just a convenient way to implement the new promise - # inhabiting the old promise's corpse. - without_promise_domain({ - value$then( - private$doResolve, - private$doReject - ) - }) + private$doChain(value) } else { - private$doResolveFinalValue(value, visible) + private$ppstate$state <- private$pstate()$resolve(value, visible) } + invisible() }, doReject = function(reason) { + force(reason) + + stopifnot(private$pstate()$status() == "pending") + if (is.promising(reason)) { - reason <- as.promise(reason) - # This then() call doesn't need promise domains; semantically, it doesn't - # really exist, as it's just a convenient way to implement the new promise - # inhabiting the old promise's corpse. - without_promise_domain({ - reason$then( - private$doResolve, - private$doReject - ) - }) + private$doChain(reason) } else { - private$doRejectFinalReason(reason) + private$ppstate$state <- private$pstate()$reject(reason) } + invisible() }, - # These "final" versions of resolve/reject are for when we've - # established that the value/reason is not itself a promise. - doResolveFinalValue = function(value, visible) { - private$value <- value - private$visible <- visible - private$state <- "fulfilled" - - later::later(function() { - lapply(private$onFulfilled, function(f) { - f(private$value, private$visible) - }) - private$onFulfilled <- list() - }) + # We were resolved or rejected with a promise. Chain ourselves onto that + # promise. + doChain = function(obj) { + p <- attr(as.promise(obj), "promise_impl", exact = TRUE) + if (identical(p, self)) { + stop("Chaining cycle detected for promise") + } + private$ppstate <- p$.subsume(private$pstate()) }, - doRejectFinalReason = function(reason) { - private$value <- reason - private$state <- "rejected" - - later::later(function() { - lapply(private$onRejected, function(f) { - private$rejectionHandled <- TRUE - f(private$value) - }) - private$onRejected <- list() - + # Call at rejection time. If the rejection is not handled by the end of the + # current tick, print a helpful warning. + checkForUnhandledPromiseError = function(reason) { + force(reason) + + if (private$isTerminal) { + # The rejection wasn't handled, but maybe it will be by the end of this + # tick (i.e. `promise_reject("boom") %...!% {}` is perfectly valid). later::later( ~ { - if (!private$rejectionHandled) { - # warning() was unreliable here + if (private$isTerminal) { + # Nobody has shown up to handle it. Print a warning. cat( file = stderr(), "Unhandled promise error: ", - reason$message, + conditionMessage(reason), "\n", sep = "" ) } } ) - }) + } } ), public = list( + initialize = function() { + private$ppstate <- PromiseStatePointer$new(PromiseStatePending$new()) + private$pstate()$register( + onRejected = list(private$checkForUnhandledPromiseError) + ) + }, # "pending", "fulfilled", "rejected" status = function() { - private$state + private$pstate()$status() + }, + .subsume = function(other_pstate) { + private$isTerminal <- FALSE + other_pstate$chain(private$pstate()) + private$ppstate }, resolve = function(value) { # Only allow this to be called once, then no-op. @@ -149,6 +283,8 @@ Promise <- R6::R6Class( invisible() }, then = function(onFulfilled = NULL, onRejected = NULL, onFinally = NULL) { + private$isTerminal <- FALSE + onFulfilled <- normalizeOnFulfilled(onFulfilled) onRejected <- normalizeOnRejected(onRejected) if (!is.function(onFinally)) { @@ -181,31 +317,7 @@ Promise <- R6::R6Class( } } - if (private$state == "pending") { - private$onFulfilled <- c( - private$onFulfilled, - list( - handleFulfill - ) - ) - private$onRejected <- c( - private$onRejected, - list( - handleReject - ) - ) - } else if (private$state == "fulfilled") { - later::later(function() { - handleFulfill(private$value, private$visible) - }) - } else if (private$state == "rejected") { - later::later(function() { - private$rejectionHandled <- TRUE - handleReject(private$value) - }) - } else { - stop("Unexpected state ", private$state) - } + private$pstate()$register(list(handleFulfill), list(handleReject)) }) invisible(promise2) @@ -219,14 +331,7 @@ Promise <- R6::R6Class( )) }, format = function() { - if (private$state == "pending") { - "" - } else { - classname <- class(private$value)[[1]] - if (length(classname) == 0) classname <- "" - - sprintf("", private$state, classname) - } + private$pstate()$format() } ) ) @@ -234,7 +339,7 @@ Promise <- R6::R6Class( normalizeOnFulfilled <- function(onFulfilled) { if (!is.function(onFulfilled)) { if (!is.null(onFulfilled)) { - warning("`onFulfilled` must be a function or `NULL`") + stop("`onFulfilled` must be a function or `NULL`") } return(NULL) } @@ -262,7 +367,7 @@ normalizeOnFulfilled <- function(onFulfilled) { normalizeOnRejected <- function(onRejected) { if (!is.function(onRejected)) { if (!is.null(onRejected)) { - warning("`onRejected` must be a function or `NULL`") + stop("`onRejected` must be a function or `NULL`") } return(NULL) } diff --git a/tests/testthat/test-domains.R b/tests/testthat/test-domains.R index 49b03a2a..d4c0024a 100644 --- a/tests/testthat/test-domains.R +++ b/tests/testthat/test-domains.R @@ -136,7 +136,7 @@ describe("Promise domains", { cd2 <- create_counting_domain(trackFinally = TRUE) p2 <- with_promise_domain(cd2, { - promise_reject(TRUE) %>% + promise_reject("bad") %>% finally( ~ { expect_identical(cd2$counts$onFinallyActive, 1L) diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index f4b96433..d441d3c9 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -34,14 +34,14 @@ describe("then()", { }) it("method ignores non-functions or NULL...", { p1 <- promise(~ resolve(1)) - expect_warning( + expect_error( { p1 <- p1$then(10) }, "`onFulfilled` must be a function or `NULL`", fixed = TRUE ) - expect_warning( + expect_error( { p1 <- p1$then(NULL) }, @@ -86,14 +86,14 @@ describe("catch()", { }) it("method ignores non-functions or NULL...", { p1 <- promise(~ resolve(1)) - expect_warning( + expect_error( { p1 <- p1$catch(10) }, "`onRejected` must be a function or `NULL`", fixed = TRUE ) - expect_warning( + expect_error( { p1 <- p1$catch(NULL) },