Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise domains broken by coro #59

Closed
jcheng5 opened this issue Nov 22, 2024 · 1 comment
Closed

Promise domains broken by coro #59

jcheng5 opened this issue Nov 22, 2024 · 1 comment

Comments

@jcheng5
Copy link
Member

jcheng5 commented Nov 22, 2024

DRAFT

Promise domains are broken when using certain coro operations (but not all of them). This means some operations using Shiny + elmer lose track about what Shiny session they belong to. tidyverse/ellmer#163

Primer on promise domains

With async programming, withr-style temporary state management won't work:

library(promises)
library(withr)
library(httr2)

perform <- async(function() {
  stopifnot(one = identical(Sys.getenv("FOO"), "1"))
  request("https://posit.co") |> req_perform_promise() |> then(\(result) {
	  stopifnot(two = identical(Sys.getenv("FOO"), "1"))
  })
})

with_envvar(list(FOO="1"), {
  perform()
})
# Prints: 'Unhandled promise error: two'

We want the effects of something like with_envvar to carry through the promise chain, until all the handlers that were bound within with_envvar are completed.

But it's not enough to simply extend the effect of with_envvar until the entire operation has completed--that is, set the env var when with_envvar() is called and delay unsetting the env var until the promise chain is completed. You can imagine two overlapping async operations that each want to set a different value for $FOO; whichever one started second would stomp on the value of the first.

Instead, we need a way to put each of the handlers in the promise chain in its own little with_envvvar. This is what promise domains let you do.

library(promises)
library(withr)
library(httr2)

perform <- async(function() {
  stopifnot(one = identical(Sys.getenv("FOO"), "1"))
  request("https://posit.co") |> req_perform_promise() |> then(\(result) {
	  stopifnot(two = identical(Sys.getenv("FOO"), "1"))
    message("success")
  })
})

envvar_domain <- function(new) {
  promises::new_promise_domain(
    wrapOnFulfilled = function(onFulfilled) {
      function(value, visible) {
        with_envvar(new, {
          onFulfilled(value, visible)
        })
      }
    },
    wrapOnRejected = function(onRejected) {
      function(reason) {
        with_envvar(new, {
          onRejected(reason)
        })
      }
    },
    wrapSync = function(expr) {
      with_envvar(new, {
        expr
      })
    }
  )
}

with_promise_domain(envvar_domain(list(FOO="1")), {
  perform()
})
# Prints: 'success'

The way promise domains work is by installing themselves into the dynamic scope exactly like a normal withr function, but the effect it has is to intercept calls to promises::then (as well as functions and operators built on top like catch, finally, %...>%, etc.). They are also viral/recursive in that, whenever one of its intercepted then calls' callbacks are invoked, they install themselves as the current promise domain so that the callbacks' then calls are also intercepted.

(The original design doc is in this gist, but the examples above are better. Also the original design doc didn't have wrapSync, which is used to wrap the expression that's directly inside with_promise_domain()--without this, whatever side effect the promise domain was intended to perform/unperform would only happen in the promise callbacks, not in the synchronous code that launches the async operation in the first place.)

Simple reprex

@jcheng5
Copy link
Member Author

jcheng5 commented Nov 25, 2024

Never mind, it was a bug in promises itself and will be fixed in rstudio/promises#110

@jcheng5 jcheng5 closed this as completed Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant