Skip to content
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.

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

Fix observeEvent stack trace stripping #4163

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Conversation

jcheng5
Copy link
Member

@jcheng5 jcheng5 commented Dec 10, 2024

Fixes #4162.

  • Unit test

Explanation

Reactive objects have a lot of machinery that we generally want to hide, so they call ..stacktraceoff.. before they begin executing and ..stacktraceon.. right before they call user-provided code.

In the specific case of observeEvent, the inner observe() call had an explicit ..stacktraceon = FALSE. Once upon a time this was correct, as a line slightly preceding it took care of turning the stack trace back on. But during the introduction of bindEvent, that previous line went away.

@jcheng5
Copy link
Member Author

jcheng5 commented Dec 10, 2024

I (lazily/expediently) added only a unit test for this specific issue. Ideally we'd have unit tests for every place in Shiny where we'd expect stack traces from:

  1. Top-level app.R/ui.R/server.R/global.R
  2. R/ subdirectory
  3. UI code
  4. UI function
  5. Server function
  6. Output renderers
  7. observe
  8. observeEvent (both eventExpr and handlerExpr)
  9. observe |> bindEvent (both eventExpr and expr)
  10. reactive
  11. eventReactive
  12. reactive |> bindEvent
  13. Module UI
  14. Module server function
  15. Download handlers
  16. session$registerDataObj? Or maybe not?
  17. Session callbacks (onFlush, onFlushed, onEnded, onSessionEnded, etc.)
  18. Bookmarking callbacks
  19. renderPlot width/height functions

I'm probably missing more than a few...

@jcheng5 jcheng5 requested a review from wch December 10, 2024 01:08
@jcheng5 jcheng5 merged commit 9a35b01 into main Dec 10, 2024
12 checks passed
@jcheng5 jcheng5 deleted the observe-event-stacktrace branch December 10, 2024 04:50
@gadenbuie gadenbuie added this to the v1.10.0 milestone Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors in observeEvent have too much stack trace hidden
3 participants