mirror of
https://github.com/rstudio/shiny.git
synced 2026-04-29 03:00:45 -04:00
3c18aca49b
* 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>