Skip to content

Reduce stack trace length #4186

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Reduce stack trace length #4186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
619a72e
Better truncate rlang and cli errors in stack trace
olivroy Feb 4, 2025
cfbc467
Add test for cli and update test + condition
olivroy Feb 4, 2025
c98e542
Merged upstream/main into stack-trsce
olivroy Feb 4, 2025
9ed4362
Add news
olivroy Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

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

* Shiny better truncates stack traces when errors occur (@olivroy #4186).

## Bug fixes

* 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)
Expand Down
2 changes: 1 addition & 1 deletion R/conditions.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ dropTrivialFrames <- function(callnames) {
# the calls--they don't add any helpful information. But only remove
# the last *contiguous* block of them, and then, only if they are the
# last thing in the calls list.
hideable <- callnames %in% c(".handleSimpleError", "h", "base$wrapOnFulfilled")
hideable <- callnames %in% c(".handleSimpleError", "h", "base$wrapOnFulfilled", "signal_abort", "handlers[[1L]]", "signalCondition", "rlang::abort", "abort", "cli::cli_abort", "cli_abort")
# What's the last that *didn't* match stop/.handleSimpleError/h?
lastGoodCall <- max(which(!hideable))
toRemove <- length(callnames) - lastGoodCall
Expand Down
99 changes: 98 additions & 1 deletion tests/testthat/_snaps/stacks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
num call loc
1 67 h
2 66 .handleSimpleError
3 65 stop
3 65 f
4 64 A [test-stacks.R#3]
5 63 B [test-stacks.R#7]
6 62 <reactive:C> [test-stacks.R#11]
Expand Down Expand Up @@ -88,3 +88,100 @@
66 2 tryCatch
67 1 try

---

Code
df
Output
num call loc
1 69 <Anonymous>
2 68 signalCondition
3 67 signal_abort
4 66 rlang::abort
Comment on lines +97 to +100
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is to get rid of these, but my approach doesn't work currently. Let me know if you have an idea on how to do this.

5 65 f
6 64 A [test-stacks.R#3]
7 63 B [test-stacks.R#7]
8 62 <reactive:C> [test-stacks.R#11]
9 42 C
10 41 renderTable [test-stacks.R#18]
11 40 func
12 39 force
13 38 withVisible
14 37 withCallingHandlers

---

Code
df
Output
num call loc
1 69 <Anonymous>
2 68 signalCondition
3 67 signal_abort
4 66 rlang::abort
5 65 f
6 64 A [test-stacks.R#3]
7 63 B [test-stacks.R#7]
8 62 <reactive:C> [test-stacks.R#11]
9 61 ..stacktraceon..
10 60 .func
11 59 withVisible
12 58 withCallingHandlers
13 57 contextFunc
14 56 env$runWith
15 55 withCallingHandlers
16 54 domain$wrapSync
17 53 promises::with_promise_domain
18 52 captureStackTraces
19 51 force
20 50 domain$wrapSync
21 49 promises::with_promise_domain
22 48 withReactiveDomain
23 47 domain$wrapSync
24 46 promises::with_promise_domain
25 45 ctx$run
26 44 self$.updateValue
27 43 ..stacktraceoff..
28 42 C
29 41 renderTable [test-stacks.R#18]
30 40 func
31 39 force
32 38 withVisible
33 37 withCallingHandlers
34 36 domain$wrapSync
35 35 promises::with_promise_domain
36 34 captureStackTraces
37 33 doTryCatch
38 32 tryCatchOne
39 31 tryCatchList
40 30 tryCatch
41 29 do
42 28 hybrid_chain
43 27 renderFunc
44 26 renderTable({ C() }, server = FALSE)
45 25 ..stacktraceon.. [test-stacks.R#17]
46 24 contextFunc
47 23 env$runWith
48 22 withCallingHandlers
49 21 domain$wrapSync
50 20 promises::with_promise_domain
51 19 captureStackTraces
52 18 force
53 17 domain$wrapSync
54 16 promises::with_promise_domain
55 15 withReactiveDomain
56 14 domain$wrapSync
57 13 promises::with_promise_domain
58 12 ctx$run
59 11 ..stacktraceoff..
60 10 isolate
61 9 withCallingHandlers [test-stacks.R#16]
62 8 domain$wrapSync
63 7 promises::with_promise_domain
64 6 captureStackTraces
65 5 doTryCatch [test-stacks.R#15]
66 4 tryCatchOne
67 3 tryCatchList
68 2 tryCatch
69 1 try

25 changes: 19 additions & 6 deletions tests/testthat/test-stacks.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
causeError <- function(full) {
causeError <- function(full, f = stop) {
A <- function() {
stop("foo")
f("foo")
}

B <- function() {
Expand Down Expand Up @@ -63,7 +63,7 @@ extractStackTrace <- function(calls,
# the calls--they don't add any helpful information. But only remove
# the last *contiguous* block of them, and then, only if they are the
# last thing in the calls list.
hideable <- callnames %in% c("stop", ".handleSimpleError", "h")
hideable <- callnames %in% c("stop", ".handleSimpleError", "h", "base$wrapOnFulfilled", "signal_abort", "handlers[[1L]]", "signalCondition", "abort", "rlang::abort", "cli::cli_abort", "cli_abort", "f")
# What's the last that *didn't* match stop/.handleSimpleError/h?
lastGoodCall <- max(which(!hideable))
toRemove <- length(calls) - lastGoodCall
Expand Down Expand Up @@ -122,20 +122,33 @@ test_that("integration tests", {
# If promises changes its internals, it can break this test on CRAN. Because
# CRAN package releases are generally not synchronized (that is, promises and
# shiny can't be updated at the same time, unless there is manual intervention
# from CRAN maintaineres), these specific test expectations make it impossible
# from CRAN maintainers), these specific test expectations make it impossible
# to release a version of promises that will not break this test and cause
# problems on CRAN.
skip_on_cran()

df <- causeError(full = FALSE)
df <- causeError(full = FALSE, f = stop)
# dumpTests(df)

expect_snapshot(df)

df <- causeError(full = TRUE)
df <- causeError(full = TRUE, f = stop)

expect_snapshot(df)
# dumpTests(df)

# Error with cli
df <- causeError(full = FALSE, f = cli::cli_abort)
# dumpTests(df)

expect_snapshot(df)

df <- causeError(full = TRUE, f = cli::cli_abort)

expect_snapshot(df)
# dumpTests(df)


})

test_that("shiny.error", {
Expand Down
Loading