Skip to content

fix(session): Send each output's progress message before it renders - #2525

Draft
jat255 wants to merge 6 commits into
jat255/2508-bookmark-on-restore-orderfrom
jat255/2508-windows-busy-indicator-check
Draft

jat255 wants to merge 6 commits into
jat255/2508-bookmark-on-restore-orderfrom
jat255/2508-windows-busy-indicator-check

Conversation

@jat255

@jat255 jat255 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

On Windows, most outputs never show their busy indicator while a synchronous render function runs. This PR sends each output's "recalculating" message before the output renders, and adds a test that measures this in the browser.

Refs #1381 (closed in 2024 by #1388, but it still occurs on main). Refs #2508, for its #1381 regression check. Stacked on #2524.

FYI, this adds a new playwright test on Windows to catch/cover the failing behavior and fix. I put it in a new job since it's conceptually different than the other jobs that already exist, but I can rope it into the other playwright suite, if desired.

Summary

Before an output renders, Shiny sends a "recalculating" message, which turns on the output's busy indicator in the browser. On Windows, a websocket write made while an earlier write is still in flight goes out only when the event loop runs again. A synchronous render function blocks the loop, so the message reached the browser together with the output's new value, and the indicator never showed.

  • A yield after the "recalculating" message lets the message go out before the render starts (shiny/session/_session.py).
  • A new Playwright test records, in the browser, when each output's "recalculating" and "recalculated" messages arrive. It fails when they arrive less than 250 ms apart for a render that blocks for 500 ms (test_busy_indicator_progress.py).
  • A new playwright-windows CI job runs the test on windows-latest, and the "PR checks" gate requires it (pytest.yaml).

Review notes

  • New CI job. The other Playwright jobs run on Ubuntu, where this test passes with or without the fix. The new job runs only the Windows-specific tests, with one Python version and Chromium. It also runs on draft PRs, because it is only one job.
  • One yield, not two. With two yields, every output sends its "recalculating" message before the first render starts, so all indicators show at once. With one yield, each indicator shows only while its own output renders, which matches R. Both pass on Windows. I used one.
  • OTel span timing changes on first load. A synchronous render function used to finish inside the session_start span. Because of the yield, it now runs just after that span ends. Its span is still a child of the session's reactive_update cycle span, and that span starts inside session_start. An async render function already behaved this way. The express-session-start OTel test asserted the old timing, so it now only waits for the first render and still checks that session_start closes.
  • The yield that Yield to give "synchronous" writes a chance to complete #1388 added after each effect run (in shiny/reactive/_reactives.py) made no measurable difference on Windows. I did not remove it, because that is outside the scope of this PR.

Testing

A temporary workflow (not in this PR) ran the test ten times for each variant on windows-latest (run):

Variant Result Smallest gap per run
This PR 10 of 10 pass 492 to 495 ms
This PR without the yield 10 of 10 fail 0.1 to 0.3 ms
main 10 of 10 fail 0.2 to 0.3 ms

An earlier run had the same result for main with the #1388 yields removed. On macOS, the test passes with or without the fix. The full pytest suite passes, and black, flake8, and pyright are clean on the changed files.

jat255 added 3 commits October 5, 2026 09:26
A Playwright test records when the browser receives each output's
"recalculating" and "recalculated" messages, and fails when they arrive
together (#1381). A one-off workflow runs it on windows-latest against
main, main without the #1388 yields (the negative control), this branch,
and this branch without its remaining yield.
On Windows, a websocket write made while an earlier write is still in flight goes out only when the event loop runs again. A synchronous renderer blocks the loop, so the output's "recalculating" message reached the client together with its value and the busy indicator never showed (#1381). A yield after the message lets it go out first.
@jat255 jat255 mentioned this pull request Oct 5, 2026
13 of 15 tasks
@jat255
jat255 added this pull request to stack #2516 October 5, 2026 13:44
jat255 added 3 commits October 5, 2026 09:49
The other Playwright jobs run on Ubuntu, where the #1381 busy-indicator test passes with or without its fix. A Windows job runs it where it can fail, and the PR gate requires it. This replaces the one-off check workflow.
An output now yields before it renders, so the first render of span_summary can run after the session_start span has closed. The test still checks that the span closes.
On Windows, `playwright install --with-deps` only adds the Media Foundation feature, which takes about 3 minutes and which headless Chromium doesn't need. A new PLAYWRIGHT_INSTALL_ARGS Makefile variable (default `--with-deps`) lets the job install only Chromium's headless shell.

This branch has not been deployed

No deployments
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.

1 participant