* test: add failing tests for render* stack trace fence coverage
Add tests for renderPlot, renderPrint, renderText, renderUI,
renderTable, and renderImage verifying that internal rendering
pipeline frames are hidden by stack trace fences.
All 6 tests fail, revealing that:
- All render functions leak `renderFunc` through the fences
- renderPlot additionally leaks `drawPlot`, `drawReactive`
- renderPrint additionally leaks `with_promise_domain`
Relates to #4357
* refactor: move stack trace test helpers to helper-stacks.R
Move extractStackTrace, cleanLocs, dumpTests, and
captureFilteredRenderTrace into helper-stacks.R so they are
available to all test files. Rename causeRenderError to
captureFilteredRenderTrace.
* fix: add stack trace fences to hide internal render pipeline frames
Add ..stacktraceoff../..stacktraceon.. fence pairs so that internal
rendering pipeline frames (renderFunc, hybrid_chain, drawPlot, etc.)
are hidden from filtered stack traces in the debugger.
- markRenderFunction: wrap renderFunc() call with ..stacktraceoff..
- createRenderFunction: wrap func() with ..stacktraceon.. to restore
visibility for user code
- renderPrint: wrap func() with ..stacktraceon.. inside promise domain
For renderPlot, the existing ..stacktraceon from installExprFunction
is sufficient once the outer ..stacktraceoff.. is in place.
Fixes#4357
* fix: Legacy datatable stack traces
* chore: Add news bullet
* chore: Just normalize path in the place that needs it
* chore: don't normalize file path
* fix: Handle srcfilealias in stack traces and telemetry
Following commit 272dda27e, which normalized paths only in the #line
directive, sourceUTF8() now creates srcfilealias objects for user code.
This broke code that assumed only package code had srcfile$original.
## How the new approach works
When sourceUTF8() wraps code with a #line directive:
```r
file <- 'app.R' # Keep original path (relative/symlink/as-typed)
lines <- c(
'..stacktraceon..({',
sprintf('#line 1 "%s"', normalizePath(file, ...)), # Normalize HERE
readLines(file),
'})'
)
src <- srcfilecopy(file, lines, isFile = TRUE) # Uses original path
expr <- parse(text = lines, srcfile = src)
```
The parser sees the #line path differs from srcfilecopy's path, so it
creates a srcfilealias with:
- srcfile$filename = absolute path (from #line, for source refs)
- srcfile$original$filename = original path (from srcfilecopy)
This gives us both: accurate source references + user-friendly paths.
## Changes made
1. Add getSrcfileFilename() helper
- Prefers $original$filename (user-typed path) when available
- Falls back to $filename (absolute) for old-style srcfile objects
- Ensures stack traces show "app.R#10" not "/abs/path/app.R#10"
2. Add isPackageFile() helper
- Checks if absolute path is under .libPaths()
- More reliable than checking for $original presence
3. Fix getCallCategories()
- Now uses isPackageFile() instead of checking $original
- User code properly categorized as "user" (bold blue in traces)
- Package code properly categorized as "pkg" (de-emphasized)
4. Update getLocs() and otel_srcref_attributes()
- Use getSrcfileFilename() to show user-friendly paths
## Benefits
- Stack traces preserve relative paths and symlinks as users typed them
- User vs package code still correctly distinguished
- Better IDE integration (paths match what user entered)
- Telemetry contains meaningful file paths
* fix: Avoid using startsWith()
* fix: Use reverse clamped cumsum for stack trace fence filtering
Replace the forward cumulative sum in stripStackTraces() with a reverse
clamped cumulative sum so that an unmatched `..stacktraceoff..` (one
with no corresponding inner `..stacktraceon..`) is a no-op. This fixes
a regression where markRenderFunction-only callers (e.g. htmlwidgets)
had their user frames hidden when called outside a reactive domain.
The new algorithm concatenates all trace segments into a single vector,
performs vectorized fence scoring, and computes visibility via the
identity: clamped_cumsum = cumsum - pmin(0, cummin(cumsum)).
Fixes#4357
* refactor: Extract skip_if_shiny_otel_tracer_is_enabled() helper
* fix: Handle srcfilealias in reactive auto-labeling
The normalizePath() in sourceUTF8() causes R to create srcfilealias
objects whose $lines is NULL, breaking rassignSrcrefToLabel() and
rexprSrcrefToLabel(). Add getSrcfileLines() helper (alongside
getSrcfileFilename()) to resolve lines from the original srcfilecopy
using srcref[7] for the correct line number.
* fix: Enforce path-boundary check in isPackageFile()
The prefix-only matching in isPackageFile() could misclassify paths
like "/usr/lib/Rcpp/..." as inside "/usr/lib/R". Normalize library
paths with a trailing slash before comparison to ensure proper
path-boundary matching.
* fix: Prefer original filename only for non-package srcfilealias
When a package is installed with keep.source.pkgs = TRUE, the
srcfilecopy original filename may point to a collated build-time path.
For package files (under .libPaths()), keep srcfile$filename to avoid
regressing stack traces and telemetry with install-time paths.
* Update R/conditions.R
---------
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
* Put action icon and label into containers
* Update snaps
* More robust test
* Don't include container if icon/label isn't specified
* `yarn build` (GitHub Actions)
* Send HTML string/deps on update; update news
---------
Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
* v1.11.1 release candidate
* `yarn build` (GitHub Actions)
* Revert actionButton()/actionLink() implementation to v1.11.0's behavior (re-introducing #4239)
* Minimal fix to address the regression in #4239
Ideally we'd fix this issue, and also get updateActionButton() working with HTML labels, but thanks to today's release of kinesis (which snapshots all of actionButton()s markup), and CRAN dragging their feet to accept our original submission (which was fine, by the way), we can't have nice things
* `yarn build` (GitHub Actions)
---------
Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
* Revert the addition of spacing between icon and label in actionButton()
* `yarn build` (GitHub Actions)
---------
Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
* Close#4239: fix front-end action button label updating logic (follow up to #3996)
* Update news
* Use a separator instead of putting markup in attributes
* `yarn build` (GitHub Actions)
* Address feedback
* Cleanup
* Refactor into a single method to split icon/label
* `yarn build` (GitHub Actions)
* Better naming
* Add some padding to the separator
* Add some unit tests for R logic
* Update NEWS.md
* Update NEWS.md
* Update NEWS.md
* Update NEWS.md
* Increase backcompat (keep same R structure when no icon is provided)
* Refine comment
---------
Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
* Use less expensive version of getCallNames() just for hashing
* Update R/conditions.R
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
---------
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
Co-authored-by: Barret Schloerke <barret@rstudio.com>
* feat: Fully reload ui/server when autoreload occurs
* chore: remove stray empty line
* chore: clean up function names and add comments
* docs: Add news item
* feat: Use {watcher} for autoreload file watching (#4185)
* feat: Use {watcher}
* chore: shikokuchuo/watcher@dev
* chore: watcher is on CRAN now
* chore: Undo air format changes
* feat: Use `shiny.autoreload.interval` for watcher latency
* chore: Simply track last time auto-reload changed
* docs: rewrite options docs for clarity
* chore: code style
* docs: global.R changes are not applied
* feat(ui/server): Autoreload also reloads global and R support files
* chore: remove outdated comment
* chore: safer comparisons
* chore: Restore legacy autoreload watcher if {watcher} not installed
* rename: autoload_r_support_if_needed()
* chore: use `rlang::is_false()`
* chore: use_build_ignore("_dev")
* Make sure spinner is visible when htmlwidget errors are visible
* Give recalculating outputs a min-height large enough to show the spinner
* tableOutput() now gets the spinner treatment
* yarn run bundle_extras
* Forward visibility hidden for all recalculating widgets, not just those with a error message (otherwise spinner won't be visible after a req())
* Update news
* Limit deep stack growth
* Improvements to deep stack trace culling
- Keep around the first deep stack trace; it may have useful
information. (We may want to change this in the future to
keep the first two stack traces, or even make it an option)
- Print out an indicator that we've elided stack traces, and
how many
* Add comments
* Add NEWS item
* Add test for unlimited deep stacks
* Code review feedback
* Code review feedback
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
* Use head() over indexing
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
* Improve unit test robustness
* Remove vector indices from snapshot
* Make stack trace stripping work across deep stacks
* Pass tests
* Try passing tests again
* Rename keep_head to retain_first_n
* Remove misleading variable assignment
* Add more comments, refine dropTrivialTestFrames
* Don't call stripStackTraces if we're not stripping
* Use deep stack deduplication instead of elision
This hopefully will avoid any potential ..stacktraceon../off..
scoring issues, and will be more useful for users. The downside
is that it's still possible to have uselessly large deep stack
traces, but at least that will only happen now if you have
manually written gigantic async/promise chains by hand or maybe
did some clever metaprogramming. The coro case should be fine.
* Add coro-based unit test
* Use rlang::hash, it's much faster
* typo
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
* Remove unnecessary logic
* Simplify/robustify reactlog version checking test
* Warn only once on call stack digest cache miss
* Super conservatively wrap appendCallStackWithDupe in try/catch
* Use more specific attribute name
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
* Remove excessively cautious try/catch
---------
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
* Avoid way too many promise domains being activated
Using `captureStackTraces` in wrapForContext is a bad idea, it
piles on a new domain every time a handler is bound.
* Use captureStackTraces, it means the same thing
* Update promises version requirement
* Add test for stack trace growth
* Simplify stack trace snapshot tests
The `category` column isn't a good candidate for snapshot
testing, as its contents vary depending on how the package
was loaded/installed. During devtools::test() or similar,
shiny package code shows up as 'user'. But during CI, it
doesn't show up as anything.
* Follow up to #4040: enable busy indicators by default
* Make our spinner invisible when wrapped inside a shinycssloaders::withSpinner() container
* Add the ability to disable/customize recalculating opacity (i.e., fade)
* Fix bug with fade not being applied correctly when the output container has no children
* `devtools::document()` (GitHub Actions)
* `yarn build` (GitHub Actions)
* Update NEWS.md
* Follow up to b7e7af: need to also rest opacity for :empty case (for initial calculation)
* Rd docs fixes/improvements
---------
Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
* chore: Enable return of dependency CSS as Sass files
Makes it possible to extract the Sass files prior to compilation for the following CSS:
* shiny
* selectize
* ionrangeslider
* daterange picker
* refactor: Take a more functional approach
* fix: missing selectizeDir
* rename: __SassLayer --> __Sass
