Consider frame complete when it has no payload

[alandipert]

Background

One day we were recording a session with shinyloadtest and we noticed that closing the browser tab did not immediately terminate the session. In normal operation, when shinyloadtest detects that the browser's websocket has closed (via httpuv WebSocketws$onClose()), it closes the "upstream" (Shiny app) websocket and records a WS_CLOSE message in the recording log. Then record_session() returns in the R console.

Instead, record_session() was behaving like the browser tab had not been closed.

We only observed this problem with Firefox, and only when the target application was RSC (and not e.g. runExample("01_hello") locally). When we tried to reproduce with Chrome, record_session() returned immediately after tab close, as we expected it to.

Using Firefox, record_session() would eventually return, about 20 seconds after tab close.

We couldn't expect users to wait that long, and we didn't want to encourage hitting Ctrl-c in the R console because that method introduces other potential difficulties.

Cause

Chrome vs Firefox

We compared Chrome and Firefox with Wireshark and determined that both browsers were sending WebSocket Close messages.

Closing the tab in Firefox resulted in the 20 second wait, but exiting Firefox completely would cause $onClose() to fire.

We concluded that Chrome's WebSocket was closing immediately (at the TCP level) and Firefox was not — WebSockets in Firefox appear to live for 20 seconds after the tab in which they were created is closed.

RSC vs local

We noticed that recording a local Shiny app worked, but recording a remote RSC app did not. Comparing traffic with Wireshark, we noticed the Close message sent in the local app case had a reason payload, 1001 - "Going Away". The Close message sent in the RSC app case had no payload.

sock.js

The primary difference between recording a local app and an RSC/SSP app is the presence of shiny-server-client and sock.js in the networking stack.

We concluded that the discrepancy in the Close message payload was accounted for by the fact that when sock.js is involved, it calls close() on the JS WebSocket, not the browser.

The version of sock.js we used registers for the browser's onunload event and closes the WebSocket without a close message: https://github.com/sockjs/sockjs-client/blob/0c70698bddcfab826c7b241ed709f69b5b0d41f7/lib/trans-websocket.js#L31

When the browser closes the WebSocket (in the local app case), it does so with the 1001 - "Going Away" payload.

Fix

Firefox's behavior (waiting 20 seconds to close the connection at the TCP layer) combined with attempting to record an app involving sock.js exposed a bug in httpuv.

Currently, onFrameComplete() is only invoked when a WebSocket message contains a payload, after that payload has been consumed.

With this change, onFrameComplete() is also invoked after the header is parsed and there is no payload.

Testing/QA notes

Set up test application

You'll need a Shiny application to test running in either SSP or RSC. This problem does not manifest against Shiny apps running from R Console or IDE because such applications don't use SockJS (Websocket abstraction library to support clients without native websocket support)

Reproduce existing bug

1. Install httpuv 1.5.1 from CRAN: devtools::install_version("httpuv", "1.5.1")
2. Install shinyloadtest from master: devtools::install_github("rstudio/shinyloadtest")
3. Start a recording of your test application by running shinyloadtest::record_session("<URL>", open_browser = FALSE) in an R console
4. Open http://localhost:8600 in Firefox
5. Interact for a few seconds with the test application
6. When you're done, close the Firefox tab
7. After 20 seconds have elapsed, you should see the recording complete in the R console.

If you use Chrome instead of Firefox, you should see the recording stop immediately. If you use Firefox but quit Firefox instead of just closing the tab, you should also see the recording stop immediately.

Confirm the fix

1. Install httpuv from Github: devtools::install_github("rstudio/httpuv")
2. Start a recording of your test application by running shinyloadtest::record_session("<URL>", open_browser = FALSE) in an R console
3. Open http://localhost:8600 in Firefox
4. Interact for a few seconds with the test application
5. When you're done, close the Firefox tab. You should notice the recording terminate immediately in the R console.

If you use Chrome instead of Firefox, you should still see the recording stop immediately. If you use Firefox but quit Firefox instead of just closing the tab, you should also still see the recording stop immediately.

[jcheng5]

Something like this could serve as a unit test:

elapsed <- NULL

srv <- httpuv::startServer("127.0.0.1", 8101, list(
  onWSOpen = function(ws) {
    open_time <- Sys.time()
    ws$onClose(function(e) {
      elapsed <<- Sys.time() - open_time
      httpuv::stopServer(srv)
    })
  }
))

# "Unnecessary" braces here to prevent `later` from attempting to
# run callbacks if this test is pasted at the console
{
  ws_client <- websocket::WebSocket$new("ws://127.0.0.1:8101")
  ws_client$onOpen(function(event) {
    ws_client$close(NA)
  })
}

while (!later::loop_empty()) {
  later::run_now()
}

testthat::expect_true(elapsed < as.difftime(0.2, units = "secs"))