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

bidirectional synchronization triggers back and forth fight between transactions #2767

Open
stefanoborini opened this issue Feb 24, 2020 · 7 comments

Comments

@stefanoborini
Copy link

stefanoborini commented Feb 24, 2020

System details

Browser Version: Chrome 80.0.3987.116 on macOS.

Output of sessionInfo():

R version 3.5.3 (2019-03-11)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Mojave 10.14.6

Matrix products: default
BLAS: /Library/Frameworks/R.framework/Versions/3.5/Resources/lib/libRblas.0.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.5/Resources/lib/libRlapack.dylib

locale:
[1] en_GB.UTF-8/en_GB.UTF-8/en_GB.UTF-8/C/en_GB.UTF-8/en_GB.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
[1] compiler_3.5.3

Example application or steps to reproduce the problem

AppModel <- function() {
  self <- reactiveValues(
      name = NULL
    )

  return(self)
}

AppModel_checkCompleteness <- function(self) {
    incomplete <- list(
      name = NULL
    )

    if (is.null(self$name) || stringr::str_length(stringr::str_trim(self$name)) == 0) {
      incomplete$name <- "You have not specified your name"
    }

    return(incomplete)
}


ui <- function() {
  view <- wellPanel(
    textInput(
      "nameText",
      "Your name and surname"),
    uiOutput("checklistUi"),
  )
  return(view)
}

server <- function(input, output, session) {                                              
  app_model <- AppModel()                                                                 
                                                                                          
  output$checklistUi <- renderUI({                                                        
    # Important. must be slow.
    Sys.sleep(2)                                                                          
                                                                                          
    completion_widget <- "Hello"                                                          
                                                                                          
    completed <- AppModel_checkCompleteness(app_model)                                    
                                                                                          
    return(completion_widget)                                                             
  })                                                                                      
                                                                                          
  observeEvent({input$nameText},                                                          
  {                                                                                       
    app_model$name <- input$nameText                                                      
  })                                                                                      
                                                                                          
  observeEvent({app_model$name},                                                          
  {                                                                                       
    updateTextInput(session, "nameText", value = app_model$name)                          
  })                                                                                      
}

shinyApp(ui = ui, server = server)

Describe the problem in detail

The problem occurs when bidirectional synchronization between a model and a UI element is needed (e.g. one has to reload model data from the disk, but also allow the user to modify it) and the transaction is slow. More details can be found on this post on the Shiny help forum.

The problem happens when the textinput ui is modified by the user, then modified again before the transaction to update the UI is completed. Taking a look at the websocket traffic, it's evident that during the second modification, a new request is sent to the server.

Later, the server replies to the first request, but this reply also contains, in the inputMessages field, the old value for the textinput. This sets the textinput to the old value, which generates a new request before the server has replied to now the second. This starts a cycle of bouncing back and forth between the two requests.

In my opinion, this is a bug that occurs because the frontend is sending a second request before the first transaction is completed. The UI should not accept any input until the current in-progress transaction goes back from busy to idle.

@stefanoborini stefanoborini changed the title bidirectional synchronization triggers bank and forth fight between transactions bidirectional synchronization triggers back and forth fight between transactions Feb 24, 2020
@stefanoborini
Copy link
Author

A possible strategy to handle this is to add a "sync" strategy for the inputs. Currently in input_rate there's throttling and debounce. One could envision a synchronized that will only send the update event if the state is not busy. It requires that the current eventhandler for busy sets a flag on the shiny app and then this state is exported and available by the synchronized input_rate.

@nteetor
Copy link

nteetor commented Feb 25, 2020

Depending on the complexity of the value you are loading you could use reactiveValues() to work around the duplication of reactivity. Assigning a reactiveValues()s element the same value will not trigger observers or reactives.

In the example below, the first time the button is pressed the state$count observer does not fire because the value is already 1.

 library(shiny)
  
  shinyApp(
    ui = fluidPage(actionButton("inc", "Increment")),
    server = function(input, output) {
      state <- reactiveValues(
        count = 1
      )
      
      observeEvent(state$count, {
        cat("Count: ", state$count, "\n")
      })
      
      observeEvent(input$inc, {
        cat("Button: ", input$inc, "\n")
        state$count <- as.numeric(input$inc)
      })
    }
  )

It sounds like the objects you are dealing with are more complex, so perhaps there's a way you could instead store a unique id in a reactiveValues()s element. Observers or reactives could watch for changes to that element and avoid the duplication.

@stefanoborini
Copy link
Author

stefanoborini commented Feb 25, 2020

@nteetor I am already using reactiveValues, and it's not the reason why this behavior occurs. The behavior occurs because:

  1. you type the value in, say "hello"
  2. an "update" message is sent from the js backend to the server carrying the value "hello".
  3. This sets the state to "busy"
  4. the model in reactiveValues is changed to have the value "hello"
  5. This generates an "inputMessages" message that sets the value "hello" back into the widget.
  6. the state goes back to "idle".

If during this transaction before the state is back to idle, another event is generated (e.g. the user types in, and the throttle of 250 ms is over), the sequence will become

  1. you type the value in, say "hello"
  2. an "update" message is sent from the js backend to the server carrying the value "hello".
  3. This sets the state to "busy"
  4. you change the value to "hello2"
  5. an "update" message is sent from the js backend to the server carrying the value "hello2"
  6. you get an "inputMessages" message as a response to the first change that sets the value "hello" back into the widget.
  7. the text widget now changes from "hello2" back to "hello", which generates a new update message
  8. the "hello2" response now comes back, which also carries a "inputMessages" with the value "hello2"
  9. repeat forever.

The problem here is that there should not be the possibility to generate another event until the first one has done the full round trip from client to server back to client. Only then, a new event can be sent. This is the way desktop UIs solve the issue, but in Shiny design the UI state and the model state are separated.

@nteetor
Copy link

nteetor commented Feb 25, 2020

Hello again, I was not trying to trivialize your problem. We've run into this duplication of reactivity a frustrating amount of times at my group, so I was hoping to help. I did illustrate my workaround poorly. Here is an updated version of the server you included above.

server <- function(input, output, session) {                                              
  state <- reactiveValues(
    name = NULL
  )
  
  app_model <- AppModel()                                                                 
  
  output$checklistUi <- renderUI({                                                        
    # Important. must be slow.
    Sys.sleep(2)                                                                          
    
    completion_widget <- "Hello"                                                          
    
    completed <- AppModel_checkCompleteness(app_model)                                    
    
    return(completion_widget)                                                             
  })   
  
  # !! important, take the input value and store it in a reactive value
  observeEvent(input$nameText, {
    state$name <- input$nameText
  })
  
  # !! important, no longer observe input$nameText, now observe state$name
  observeEvent(state$name, {     
    cat("Updating app model name\n")
    app_model$name <- input$nameText                                                      
  })                                                                                      
  
  observeEvent(app_model$name, { 
    cat("Updating browser text input\n")
    updateTextInput(session, "nameText", value = app_model$name)                          
  })                                                                                      
}

Before my changes to the server I did observe the problem you described. Afterwards I no longer saw the repeated updates.

@stefanoborini
Copy link
Author

stefanoborini commented Feb 26, 2020

@nteetor I tried your solution but I see no change. I suspect that what happens is that by adding a variable, you introduce enough delay for the transaction to complete.

In desktop UI programming the loop is always closed and extinguished because you can't process a new event before the old one has been fully processed.

@nteetor
Copy link

nteetor commented Feb 26, 2020

I’m sorry to hear that. I’m eager then for a more comprehensive solution from others or the shiny team.

@Delta-Kappa
Copy link

Hi @stefanoborini ,

tricky problem, which I also keep coming across. I'm still looking for a better solution, but for now you could do this:
You have to remember if the update of your app_model$name was triggered from the change in input$nameText. Then you can prevent the updateTextInput.
A compact way is by setting an attribute, e.g. .update_input:

observeEvent({input$nameText},  
{
    app_model$name <- structure(input$nameText, .update_input=FALSE)
})                                                                                      
  
observeEvent({app_model$name},  
{        
    if (attr(app_model$name, ".update_input") && TRUE) {
        updateTextInput(session, "nameText", value = app_model$name)                          
    }
})           

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

No branches or pull requests

3 participants