Skip to content

Commit 634b1c7

Browse files
jcheng5cpsievert
andauthored
Don't kill the session when a debounced/throttled reactive expr errors (#3624)
* Don't kill the session when a debounced/throttled reactive expr errors Fixes #3581 * Update NEWS with PR number Co-authored-by: Carson Sievert <[email protected]>
1 parent d4527cd commit 634b1c7

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ shiny development
4343

4444
* Closed rstudio/shinytest2#222: When restoring a context (i.e., bookmarking) from a URL, Shiny now better handles a trailing `=` after `_inputs_` and `_values_`. (#3648)
4545

46+
* Closed #3581: Errors in throttled/debounced reactive expressions cause session to exit. (#3624)
47+
4648

4749
shiny 1.7.1
4850
===========

R/reactives.R

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2472,11 +2472,11 @@ debounce <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai
24722472

24732473
# Ensure r() is called only after setting firstRun to FALSE since r()
24742474
# may throw an error
2475-
r()
2475+
try(r(), silent = TRUE)
24762476
return()
24772477
}
24782478
# This ensures r() is still tracked after firstRun
2479-
r()
2479+
try(r(), silent = TRUE)
24802480

24812481
# The value (or possibly millis) changed. Start or reset the timer.
24822482
v$when <- getDomainTimeMs(domain) + millis()
@@ -2509,7 +2509,7 @@ debounce <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai
25092509
# commenting it out and studying the unit test failure that results.
25102510
primer <- observe({
25112511
primer$destroy()
2512-
er()
2512+
try(er(), silent = TRUE)
25132513
}, label = "debounce primer", domain = domain, priority = priority)
25142514

25152515
er
@@ -2551,7 +2551,7 @@ throttle <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai
25512551
}
25522552

25532553
# Responsible for tracking when f() changes.
2554-
observeEvent(r(), {
2554+
observeEvent(try(r(), silent = TRUE), {
25552555
if (v$pending) {
25562556
# In a blackout period and someone already scheduled; do nothing
25572557
} else if (blackoutMillisLeft() > 0) {

tests/testthat/test-reactives.R

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,28 @@ test_that("can access reactive values directly", {
1313
y <- reactive(x1() + x2$a)
1414
expect_equal(y(), 4)
1515
})
16+
17+
test_that("errors in throttled/debounced reactives are catchable", {
18+
reactiveConsole(TRUE)
19+
on.exit(reactiveConsole(FALSE))
20+
21+
# In Shiny 1.7 and earlier, if a throttled/debounced reactive threw an error,
22+
# it would cause internal observers used by the implementations of
23+
# debounce/throttle to error, which would kill the session. The correct
24+
# behavior is to only expose the error to consumers of the throttled/debounced
25+
# reactive.
26+
27+
r <- reactive({
28+
stop("boom")
29+
})
30+
31+
rd <- r %>% debounce(1000)
32+
rt <- r %>% throttle(1000)
33+
34+
observe({
35+
try(rd(), silent = TRUE)
36+
try(rt(), silent = TRUE)
37+
})
38+
39+
expect_silent(flushReact())
40+
})

0 commit comments

Comments
 (0)