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

[Bug]: App is "frozen" when reactive() data is not passed #1307

Closed
gogonzo opened this issue Aug 12, 2024 · 3 comments · Fixed by #1341
Closed

[Bug]: App is "frozen" when reactive() data is not passed #1307

gogonzo opened this issue Aug 12, 2024 · 3 comments · Fixed by #1341
Assignees
Labels
bug Something isn't working core

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Aug 12, 2024

What happened?

When passing data as (unevaluated) eventReactive to srv_teal directly then the teal-app looks like frozen. Propose a better solution for this.

app code
options(
  teal.log_level = "TRACE",
  teal.show_js_log = TRUE,
  # teal.bs_theme = bslib::bs_theme(version = 5),
  shiny.bookmarkStore = "server"
)
library(scda)
pkgload::load_all("teal")
# pkgload::load_all("teal.slice")

ui_data <- function(id) {
  ns <- NS(id)
  tagList(
    actionButton(ns("submit"), label = "Submit to run")
  )
}

srv_data <- function(id, ...) {
  moduleServer(id, function(input, output, session) {
    eventReactive(input$submit, {
      data <- teal_data() |>
        within({
          logger::log_trace("Loading data")
          ADSL <- scda::synthetic_cdisc_data("latest")$adsl
          ADTTE <- scda::synthetic_cdisc_data("latest")$adtte
          iris <- iris
        })

      join_keys(data) <- default_cdisc_join_keys[c("ADSL", "ADTTE")]

      data
    })
  })
}

modules <- modules(
  teal.modules.general::tm_data_table("Data Table"),
  example_module("Example Module", datanames = "ADTTE"),
  module(
    ui = function(id) {
      ns <- NS(id)
      tagList(
        tableOutput(ns("filter_summary"))
      )
    },
    server = function(id, datasets) {
      moduleServer(id, function(input, output, session) {
        output$filter_summary <- renderTable({
          datasets$get_filter_overview(datanames = datasets$datanames())
        })
      })
    }
  )
)

shinyApp(
  ui = function(request) {
    fluidPage(
      ui_data("data"),
      ui_teal(id = "teal", modules = modules)
    )
  },
  server = function(input, output, session) {
    data_rv <- srv_data("data", data = data, modules = modules)
    srv_teal(id = "teal", data = data_rv, modules = modules)
  }
)
image
@gogonzo gogonzo added the bug Something isn't working label Aug 12, 2024
@gogonzo gogonzo added this to the teal transform milestone Aug 12, 2024
@gogonzo gogonzo added the core label Aug 12, 2024
@m7pr
Copy link
Contributor

m7pr commented Aug 12, 2024

It's not that bad, you can see a huge button with Submit to run. So even though you can click between modules, it looks like broken, but there is another button that triggers the data pull. Also when data is fetched the Computing... progress is visible.

We could disable the ability to go through modules, and grey them out, leaving only the Submit to run being the only active button. Or we can display a message in the module saying that data is empty?

@gogonzo
Copy link
Contributor Author

gogonzo commented Aug 12, 2024

@m7pr in the range of all the issue we solved "it is not bad", but inconsistent with teal_data_module which disables and greys-out tabs on init and enable them when initial data is provided.

@donyunardi
Copy link
Contributor

donyunardi commented Aug 12, 2024

Related to #1308

Acceptance Criteria

  • tabs are disabled and hidden with message when reactive teal_data is not executed

@vedhav vedhav self-assigned this Aug 14, 2024
@vedhav vedhav linked a pull request Aug 28, 2024 that will close this issue
@gogonzo gogonzo linked a pull request Sep 17, 2024 that will close this issue
vedhav added a commit that referenced this issue Oct 3, 2024
Alternative solution to #1330. Closes #1304, #1307, and #1308

1. When teal_data_module fails, then teal-module-tabs are disabled. When
teal_data_module returns teal_data again teal-module-tabs are enabled
2. When reactive data passed directly to srv_teal fails, then the whole
tab-panel is hidden and error message is shown. Warning messages are
displayed over tab-panel.
3. when teal_transform_module fails then following
teal_transform_module(s) show generic message that something was wrong.
Reason for this is the same as (3).
4. when teal_transform_module fails then teal-module output is disabled
and generic failure message is shown in the main panel. We decided to
show a generic failure message as "real failure message" should be only
shown in the place where error occurred to no cause confusion.
5. failing teal_data_module/teal_transform_module fallbacks to previous
valid data (see exaplanation below)

The most important part of the implementation is that when
teal_data_module fails then it return the previous valid data (i.e. it
return unchanged data). This means that failure doesn't trigger
downstream reactivity and we don't need to deal with `data` input as
error. In other words, this implementation halts reactivity when
something goes wrong.
When something goes wrong, teal-module-output is hidden and instead
error message is displayed.

Also, I've moved `data` completely away from `ui` and now if there is
`teal_data_module` then data-tab is added dynamically.

<details>
<summary>app w/ teal_data_module</summary>

```r
options(
  teal.log_level = "TRACE",
  teal.show_js_log = TRUE,
  # teal.bs_theme = bslib::bs_theme(version = 5),
  shiny.bookmarkStore = "server"
)

# pkgload::load_all("teal.data")
pkgload::load_all("teal")

make_data <- function(datanames = c("ADSL", "ADTTE")) {
  data_obj <- teal.data::teal_data()
  if ("ADSL" %in% datanames) {
    data_obj <- within(data_obj, ADSL <- teal.data::rADSL)
  }
  if ("ADTTE" %in% datanames) {
    data_obj <- within(data_obj, ADTTE <- teal.data::rADTTE)
  }
  join_keys(data_obj) <- default_cdisc_join_keys[datanames]
  teal.data::datanames(data_obj) <- datanames
  data_obj
}

trans <- list(
  teal_transform_module(
    ui = function(id) {
      ns <- NS(id)
      tagList(
        selectizeInput(
          ns("errortype"),
          label = "Error Type",
          choices = c(
            "ok", "insufficient datasets", "no data",
            "qenv.error", "error in reactive", "validate error", "silent.shiny.error", "not a reactive"
          )
        )
      )
    },
    server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        logger::log_trace("example_module_transform2 initializing.")
        reactive({
          switch(input$errortype,
            ok = data(),
            `insufficient datasets` = teal:::.subset_teal_data(data(), "ADSL"),
            `no data` = teal_data(),
            qenv.error = within(teal_data(), stop("\nthis is qenv.error in teal_transform_module\n")),
            `error in reactive` = stop("\nerror in a reactive in teal_transform_module\n"),
            `validate error` = validate(need(FALSE, "\nvalidate error in teal_transform_module\n")),
            `silent.shiny.error` = req(FALSE)
          )
        })
      })
    }
  )
)

data <- teal_data_module(
  once = FALSE,
  ui = function(id) {
    ns <- NS(id)
    tagList(
      selectizeInput(
        ns("errortype"),
        label = "Error Type",
        choices = c(
          "ok", "insufficient datasets", "no data",
          "qenv.error", "error in reactive", "validate error", "silent.shiny.error", "not a reactive"
        )
      ),
      actionButton(ns("submit"), "Go!")
    )
  },
  server = function(id, ...) {
    moduleServer(id, function(input, output, session) {
      logger::log_trace("example_module_transform2 initializing.")
      eventReactive(input$submit, {
        switch(input$errortype,
          ok = make_data(),
          `insufficient datasets` = make_data(datanames = "ADSL"),
          `no data` = teal_data(),
          qenv.error = within(data(), stop("\nthis is qenv.error in teal_data_module\n")),
          `error in reactive` = stop("\nerror in a reactive in teal_data_module\n"),
          `validate error` = validate(need(FALSE, "\nvalidate error in teal_data_module\n")),
          `silent.shiny.error` = req(FALSE)
        )
      })
    })
  }
)

app <- teal::init(
  data = data,
  modules = list(
    example_module("mod-1", transformers = c(trans, trans, trans), datanames = c("ADSL", "ADTTE")),
    example_module("mod-2", transformers = trans, datanames = c("ADSL", "ADTTE")),
    module(
      label = "I was made to annoy you",
      ui = function(id) NULL,
      server = function(id, data) {
        moduleServer(id, function(input, output, session) {
          observe({
            teal.data::datanames(data())
            ADSL <- data()[["ADSL"]]
            ADSL$AGE
          })

          observeEvent(data(), {
            print(data()[["ADSL"]]$SEX)
          })
        })
      },
      datanames = "ADSL"
    )
  ),
  filter = teal_slices(
    teal_slice("ADSL", "SEX"),
    teal_slice("ADSL", "AGE", selected = c(18L, 65L)),
    teal_slice("ADTTE", "PARAMCD", selected = "CRSD"),
    include_varnames = list(
      ADSL = c("SEX", "AGE")
    )
  )
)

runApp(app)

```

</details>

<details>
<summary>app wrapped</summary>

```r
options(
  teal.log_level = "TRACE",
  teal.show_js_log = TRUE,
  # teal.bs_theme = bslib::bs_theme(version = 5),
  shiny.bookmarkStore = "server"
)
library(scda)
pkgload::load_all("teal")

make_data <- function(datanames = c("ADSL", "ADTTE")) {
  data_obj <- teal.data::teal_data()
  if ("ADSL" %in% datanames) {
    data_obj <- within(data_obj, ADSL <- teal.data::rADSL)
  }
  if ("ADTTE" %in% datanames) {
    data_obj <- within(data_obj, ADTTE <- teal.data::rADTTE)
  }
  join_keys(data_obj) <- default_cdisc_join_keys[datanames]
  teal.data::datanames(data_obj) <- datanames
  data_obj
}

ui_data <- function(id) {
  ns <- NS(id)
  tagList(
    selectizeInput(
      ns("errortype"),
      label = "Error Type",
      choices = c(
        "ok", "insufficient datasets", "no data",
        "qenv.error", "error in reactive", "validate error", "silent.shiny.error", "not a reactive"
      )
    ),
    actionButton(ns("submit"), "Go!")
  )
}

srv_data <- function(id, ...) {
  moduleServer(id, function(input, output, session) {
    logger::log_trace("example_module_transform2 initializing.")
    eventReactive(input$submit, {
      switch(input$errortype,
        ok = make_data(),
        `insufficient datasets` = make_data(datanames = "ADSL"),
        `no data` = teal_data(),
        qenv.error = within(data(), stop("\nthis is qenv.error in teal_data_module\n")),
        `error in reactive` = stop("\nerror in a reactive in teal_data_module\n"),
        `validate error` = validate(need(FALSE, "\nvalidate error in teal_data_module\n")),
        `silent.shiny.error` = req(FALSE)
      )
    })
  })
}

modules <- modules(
  teal.modules.general::tm_data_table("Data Table"),
  example_module("Example Module", datanames = "ADTTE"),
  module(
    ui = function(id) {
      ns <- NS(id)
      tagList(
        tableOutput(ns("filter_summary"))
      )
    },
    server = function(id, datasets) {
      moduleServer(id, function(input, output, session) {
        output$filter_summary <- renderTable({
          datasets$get_filter_overview(datanames = datasets$datanames())
        })
      })
    }
  )
)

shinyApp(
  ui = function(request) {
    fluidPage(
      ui_data("data"),
      ui_teal(id = "teal", modules = modules)
    )
  },
  server = function(input, output, session) {
    data_rv <- srv_data("data", data = data, modules = modules)
    srv_teal(id = "teal", data = data_rv, modules = modules)
  }
)

```

</details>

---------

Signed-off-by: Vedha Viyash <[email protected]>
Signed-off-by: Marcin <[email protected]>
Co-authored-by: vedhav <[email protected]>
Co-authored-by: Vedha Viyash <[email protected]>
Co-authored-by: m7pr <[email protected]>
Co-authored-by: Marcin <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment