Skip to content

fix(ssr): make the render timeout cover rendering - #174

Open
bobzhang wants to merge 1 commit into
mainfrom
fix/ssr-render-timeout
Open

fix(ssr): make the render timeout cover rendering#174
bobzhang wants to merge 1 commit into
mainfrom
fix/ssr-render-timeout

Conversation

@bobzhang

@bobzhang bobzhang commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #173.

App::render's timeout could never fire. All of SSR's async work — graph evaluation, drain_task, the HTTP fetches create_resource issues — happens inside @runtime.SSRHost(...), on the line before with_timeout. What with_timeout wrapped was render_to_string, declared pub fn rather than pub async fn, so it has no suspension point and the timer can never preempt it.

This moves the SSRHost construction inside the timeout, where the awaiting actually is, and keeps defer host.cleanup() in the same closure so it still runs on both the success and the cancelled path.

 let (origin, path) = split_url(url)
-let host = @runtime.SSRHost(origin~, path~, () => {
-  let output = (self.builder)()
-  output.0.map(html => html.0)
-})
-defer host.cleanup()
-let html = @async.with_timeout(timeout, () => {
-  host.render_to_string(head.map(html => html.to_virtual_dom()))
-})
-html
+@async.with_timeout(timeout, () => {
+  let host = @runtime.SSRHost(origin~, path~, () => {
+    let output = (self.builder)()
+    output.0.map(html => html.0)
+  })
+  defer host.cleanup()
+  host.render_to_string(head.map(html => html.to_virtual_dom()))
+})

Verification

Probe in #173 — render against an endpoint that sleeps 2s, with timeout=100:

result
rabbita@0.15.6 RETURNED (no raise) after ~2s, with the fetched data
this branch RAISED: TimeoutError

Cancellation does not leak the host: with SSRHost::cleanup instrumented with a println, the timeout path prints [probe] SSRHost::cleanup ran before the raise.

  • moon check --target native --deny-warn — pass
  • moon check --target wasm --deny-warn — pass
  • moon test --target native — 86 passed, unchanged

Two things I deliberately did not do

No regression test. @cmd.perform and @cmd.effect are #cfg(target="js"), so the native async-command surface is effectively HTTP and a test needs a live server. moon test is also commented out in check.yml, so it would not run in CI anyway. Rather than add a timing-sensitive test nobody runs, the reproduction is written out in full in #173 so it can be verified independently.

No version bump. Left at 0.15.6 so the bump can be made at release time rather than going stale while this is reviewed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AmoyPMELemz5hdNNdXyAkm

`App::render`'s `timeout` documents "Throws `@async.TimeoutError` if
rendering does not complete within `timeout` milliseconds", but it could
never fire.

All of SSR's async work — graph evaluation, command draining, the HTTP
fetches `create_resource` issues — happens inside `@runtime.SSRHost(...)`,
on the line *before* `with_timeout`. What `with_timeout` wrapped was
`render_to_string`, which is declared `pub fn`, not `pub async fn`: it has
no suspension point, so the timer can never preempt it. A hanging SSR fetch
hung forever no matter what `timeout` said.

Wrap the `SSRHost` construction too, which is where the awaiting is.
`defer host.cleanup()` moves inside the same closure so it still runs on
both the success and the cancelled path.

Verified with a probe that renders against an endpoint that sleeps 2s,
with `timeout=100`:

  before  RETURNED (no raise), after ~2s, with the fetched data
  after   RAISED: TimeoutError

and, with `SSRHost::cleanup` instrumented, cleanup still runs when the
timeout cancels the render.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmoyPMELemz5hdNNdXyAkm
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.

App::render(timeout=) never fires: with_timeout wraps the synchronous step, not the async one

1 participant