Skip to content

Commit ff5ef52

Browse files
jcheng5cpsievert
andauthored
Fix #3250 (#3602)
* Fix #3250 pruneStackTrace was interacting badly with dplyr errors. I'm still not sure what causes these new cases, but the new behavior seems to be much better, with no downside that I can think of. * Fix existing unit tests * Update news Co-authored-by: Carson Sievert <[email protected]>
1 parent 634b1c7 commit ff5ef52

File tree

3 files changed

+37
-23
lines changed

3 files changed

+37
-23
lines changed

NEWS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ 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)
46+
* Closed #3581: Errors in throttled/debounced reactive expressions no longer cause the session to exit. (#3624)
47+
48+
* Closed #3250:`{rlang}`/`{tidyeval}` conditions (i.e., warnings and errors) are no longer filtered from stack traces. (#3602)
4749

4850

4951
shiny 1.7.1

R/conditions.R

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,17 @@ pruneStackTrace <- function(parents) {
421421
# Loop over the parent indices. Anything that is not parented by current_node
422422
# (a.k.a. last-known-good node), or is a dupe, can be discarded. Anything that
423423
# is kept becomes the new current_node.
424+
#
425+
# jcheng 2022-03-18: Two more reasons a node can be kept:
426+
# 1. parent is 0
427+
# 2. parent is i
428+
# Not sure why either of these situations happen, but they're common when
429+
# interacting with rlang/dplyr errors. See issue rstudio/shiny#3250 for repro
430+
# cases.
424431
include <- vapply(seq_along(parents), function(i) {
425-
if (!is_dupe[[i]] && parents[[i]] == current_node) {
432+
if ((!is_dupe[[i]] && parents[[i]] == current_node) ||
433+
parents[[i]] == 0 ||
434+
parents[[i]] == i) {
426435
current_node <<- i
427436
TRUE
428437
} else {
Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
1-
capture <- function() {
2-
list(
3-
calls = sys.calls(),
4-
parents = sys.parents()
5-
)
6-
}
1+
foo <- function() {
2+
capture <- function() {
3+
list(
4+
calls = sys.calls(),
5+
parents = sys.parents()
6+
)
7+
}
78

8-
capture_1 <- function() {
9-
capture()
10-
}
9+
capture_1 <- function() {
10+
capture()
11+
}
1112

12-
capture_2 <- function() {
13-
capture_1()
14-
}
13+
capture_2 <- function() {
14+
capture_1()
15+
}
1516

16-
res <- do.call(
17-
identity,
18-
list(
19-
identity(capture_2())
17+
do.call(
18+
identity,
19+
list(
20+
identity(capture_2())
21+
)
2022
)
21-
)
22-
res$calls <- tail(res$calls, 5)
23-
res$parents <- tail(res$parents - (length(res$parents) - 5), 5)
23+
}
24+
res <- foo()
25+
res$calls <- tail(res$calls, 6)
26+
res$parents <- tail(res$parents - (length(res$parents) - 6), 6)
2427

2528
describe("stack pruning", {
2629
it("passes basic example", {
27-
expect_equal(pruneStackTrace(res$parents), c(F, F, T, T, T))
28-
expect_equal(lapply(list(res$parents), pruneStackTrace), list(c(F, F, T, T, T)))
30+
expect_equal(pruneStackTrace(res$parents), c(T, F, F, T, T, T))
31+
expect_equal(lapply(list(res$parents), pruneStackTrace), list(c(T, F, F, T, T, T)))
2932
})
3033
})

0 commit comments

Comments
 (0)