diff --git a/CHANGELOG.md b/CHANGELOG.md index 06ef394..d13807e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ Retention runs on its own thread, so a slow pass cannot delay replacing a crashed role, and a failed pass retries at monitor cadence with a doubling backoff rather than deferring for the whole interval. +- Batch component refreshes on reconnect. A reconnecting subscription refreshed + every stale component individually, ignoring the batches those components + declared, so a page with twenty batched components issued twenty requests + instead of one. That happens at the worst moment: a server restart reconnects + every client at once. Reconnect now shares the batching the live invalidation + path uses. +- Cover the reconnect burst in the browser suite: convergence of batched and + unbatched components, an inert replay of an already-applied revision, + cancellation of the request left in flight by the drop, incarnation ordering + after a destroy and recreate, and payload delivery exactly once per revision. - Add Ruby 4.0 to the compatibility matrix, which now covers Ruby 3.3, 3.4, and 4.0 against Rails 8.0 and 8.1. diff --git a/docs/realtime.md b/docs/realtime.md index 3da2e4d..d141623 100644 --- a/docs/realtime.md +++ b/docs/realtime.md @@ -393,6 +393,11 @@ ordered commits within one incarnation. Out-of-order invalidations at or below the last transmitted pair are ignored. The durable state row remains source of truth. +Stale components that share a `batch:` are refreshed together, exactly as a +live invalidation refreshes them, so reconnecting costs one request per batch +rather than one per component. That matters most on a restart, when every +client reconnects at once. + The component endpoint rejects a requested revision newer than the committed snapshot. This is a final server-side guard; browser safety primarily comes from monotonic channel filtering plus replace-frame detachment or morph diff --git a/docs/roadmap.md b/docs/roadmap.md index 0720e27..3293881 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -52,7 +52,10 @@ - A JavaScript suite covering every browser module, run in CI with Node's test runner and jsdom, plus a browser suite running the same modules against real Chromium and a real Turbo build, with every GitHub Actions reference pinned to - a commit SHA + a commit SHA. The browser suite covers the reconnect burst: convergence of + batched and unbatched components, an inert replay of an applied revision, + cancellation of the request left in flight by the drop, incarnation ordering + after a destroy and recreate, and payload delivery exactly once per revision ## Partially implemented @@ -73,7 +76,10 @@ implemented; application-directed Turbo append intents are not. Batch coalescing happens in the browser rather than the broadcast executor, so one commit still sends one Action Cable message per changed observable even - though it costs one browser request. + though it costs one browser request. Reconnect convergence previously + bypassed batching entirely, issuing one request per stale component at the + moment a restart reconnects every client at once; it now shares the batching + the live invalidation path uses. - Backpressure: mailbox/payload/state/result caps and fair yields exist; distributed per-actor rate limits and global admission control do not. - Administration: actor and dead-letter views plus policy hooks exist; richer @@ -84,7 +90,7 @@ ## Next milestones 1. Add result lookup by request ID and broader deadlock retry classification. -2. Add Turbo append intents and expand reconnect coverage in a full browser. +2. Add Turbo append intents. 3. Add distributed rate limits, global admission hooks, and cache-capacity eviction. 4. Expand security scanning beyond the Brakeman scan, such as dependency diff --git a/lib/solid_objects/component_subscriptions.rb b/lib/solid_objects/component_subscriptions.rb index 5dc502a..077bb6a 100644 --- a/lib/solid_objects/component_subscriptions.rb +++ b/lib/solid_objects/component_subscriptions.rb @@ -52,26 +52,19 @@ def refreshes_for(invalidation) registration.dependencies.include?(observable_name) && newer_revision?(registration.dom_id, instance_id, revision) end - batched, individual = changed.partition(&:batch) - streams = individual.map { |registration| refresh(registration, instance_id, revision) } - batched.group_by(&:batch).each_value do |group| - group.each { |registration| record_revision(registration, instance_id, revision) } - streams << TurboStreamRenderer.batch_refresh(group, instance_id, revision) - end - streams + refresh_streams(changed, instance_id, revision) end # @rbs (ActorSnapshot) -> Array[String] def reconnect_refreshes(snapshot) - registrations.filter_map do |registration| - next unless newer_revision?( + stale = registrations.select do |registration| + newer_revision?( registration.dom_id, snapshot.instance_id, snapshot.revision ) - - refresh(registration, snapshot.instance_id, snapshot.revision) end + refresh_streams(stale, snapshot.instance_id, snapshot.revision) end class << self @@ -91,6 +84,21 @@ def validate_identity!(registration, reference) attr_reader :registrations, :revisions + # Live invalidations and reconnect replays share this, so a reconnecting + # client pays the same number of requests a connected one does. + # @rbs (Array[ComponentRegistration], Integer, Integer) -> Array[String] + def refresh_streams(changed, instance_id, revision) + batched, individual = changed.partition(&:batch) + streams = individual.map do |registration| + refresh(registration, instance_id, revision) + end + batched.group_by(&:batch).each_value do |group| + group.each { |registration| record_revision(registration, instance_id, revision) } + streams << TurboStreamRenderer.batch_refresh(group, instance_id, revision) + end + streams + end + # @rbs (ComponentRegistration, Integer, Integer) -> void def record_revision(registration, instance_id, revision) revisions[registration.dom_id] = [ instance_id, revision ] diff --git a/sig/generated/lib/solid_objects/component_subscriptions.rbs b/sig/generated/lib/solid_objects/component_subscriptions.rbs index 8b4746c..ff0557d 100644 --- a/sig/generated/lib/solid_objects/component_subscriptions.rbs +++ b/sig/generated/lib/solid_objects/component_subscriptions.rbs @@ -31,6 +31,11 @@ module SolidObjects attr_reader revisions: untyped + # Live invalidations and reconnect replays share this, so a reconnecting + # client pays the same number of requests a connected one does. + # @rbs (Array[ComponentRegistration], Integer, Integer) -> Array[String] + def refresh_streams: (Array[ComponentRegistration], Integer, Integer) -> Array[String] + # @rbs (ComponentRegistration, Integer, Integer) -> void def record_revision: (ComponentRegistration, Integer, Integer) -> void diff --git a/test/browser/browser_test_helper.mjs b/test/browser/browser_test_helper.mjs index 1d3380d..329584f 100644 --- a/test/browser/browser_test_helper.mjs +++ b/test/browser/browser_test_helper.mjs @@ -64,7 +64,20 @@ export async function openPage(origin) { const browser = await chromium.launch() const context = await browser.newContext() const page = await context.newPage() - await page.goto(`${origin}/`) - await page.waitForFunction(() => Boolean(customElements.get("solid-objects-refresh"))) + await loadPage(page, origin) return { browser, page } } + +// The browser modules keep applied revisions in module scope, so a test that +// needs to start from a lower revision than the previous one left behind has to +// reload rather than only reset the DOM. +export async function loadPage(page, origin) { + await page.goto(`${origin}/`) + await page.waitForFunction(() => + Boolean( + customElements.get("solid-objects-refresh") && + customElements.get("solid-objects-batch-refresh") && + customElements.get("solid-objects-payload") + ) + ) +} diff --git a/test/browser/reconnect.test.mjs b/test/browser/reconnect.test.mjs new file mode 100644 index 0000000..d1befe1 --- /dev/null +++ b/test/browser/reconnect.test.mjs @@ -0,0 +1,334 @@ +import assert from "node:assert/strict" +import { test, before, after, beforeEach, afterEach } from "node:test" +import { startServer, openPage, loadPage, frameHtml } from "./browser_test_helper.mjs" + +// A reconnecting subscription replays the current state as a burst of refresh +// and payload elements. These cover what the channel tests cannot: whether that +// burst converges the live document, and whether a replay of something already +// applied leaves the page alone. +let server +let origin +let browser +let page + +const batches = new Map() +const components = new Map() +const cancelled = new Set() +const gates = [] + +function releaseGates() { + while (gates.length) gates.pop()() +} + +before(async () => { + const started = await startServer({ + routes: { + "/solid_objects/components/batch": async (request, response, url) => { + const token = url.searchParams.get("token") + // A client abort closes the socket before the response is written, so + // this is where a cancelled request becomes observable to a test. + response.on("close", () => { + if (!response.writableEnded) cancelled.add(token) + }) + const entry = batches.get(token) + if (entry) entry.requested = true + await entry?.gate + if (response.writableEnded || response.destroyed) return + + response.writeHead(200, { "Content-Type": "application/json" }) + response.end(JSON.stringify({ frames: entry?.frames ?? [] })) + }, + "/solid_objects/components": (request, response, url) => { + response.writeHead(200, { "Content-Type": "text/html" }) + response.end(components.get(url.searchParams.get("token")) ?? "") + } + } + }) + server = started.server + origin = started.origin + const opened = await openPage(origin) + browser = opened.browser + page = opened.page +}) + +after(async () => { + releaseGates() + await browser?.close() + server?.close() +}) + +afterEach(() => { + releaseGates() +}) + +beforeEach(async () => { + batches.clear() + components.clear() + cancelled.clear() + await loadPage(page, origin) +}) + +async function waitFor(condition, message) { + const deadline = Date.now() + 5000 + while (Date.now() < deadline) { + if (condition()) return + await new Promise((resolve) => setTimeout(resolve, 20)) + } + assert.fail(message) +} + +// A gate a test forgets to release, because an assertion failed before it got +// there, leaves its route awaiting forever and turns a reported failure into a +// hung suite. Every gate is released when the test that made it finishes. +function gate() { + let release + const promise = new Promise((resolve) => { + release = resolve + }) + gates.push(release) + return { promise, release } +} + +function frame(target, revision, body) { + return { + target, + revision, + refresh_method: "morph", + html: frameHtml(target, revision, body) + } +} + +// Mirrors what ComponentSubscriptions transmits: one element per batch, +// carrying every stale target in that batch. +async function replayBatch({ revision, token, targets }) { + await page.evaluate(({ revision, token, targets }) => { + const element = document.createElement("solid-objects-batch-refresh") + element.dataset.batch = "playmat" + element.dataset.revision = revision + element.dataset.targets = targets.join(" ") + const tokens = targets.map((target) => `tokens[]=${target}`).join("&") + element.dataset.source = + `/solid_objects/components/batch?token=${token}&${tokens}` + document.getElementById("solid-objects-scope").append(element) + }, { revision, token, targets }) +} + +async function replayComponent({ target, token }) { + await page.evaluate(({ target, token }) => { + const element = document.createElement("solid-objects-refresh") + element.dataset.target = target + element.dataset.source = `/solid_objects/components?token=${token}` + element.dataset.refreshMethod = "morph" + document.getElementById("solid-objects-scope").append(element) + }, { target, token }) +} + +async function replayPayload({ name, revision, payload }) { + await page.evaluate(({ name, revision, payload }) => { + const element = document.createElement("solid-objects-payload") + element.dataset.name = name + element.dataset.revision = revision + element.textContent = JSON.stringify(payload) + document.getElementById("solid-objects-scope").append(element) + }, { name, revision, payload }) +} + +function textOf(target) { + return page.evaluate( + (target) => document.getElementById(target).textContent, + target + ) +} + +function revisionOf(target) { + return page.evaluate( + (target) => document.getElementById(target).dataset.solidObjectsRevision, + target + ) +} + +test("a reconnect converges every stale component in one request", async () => { + batches.set("reconnect", { + frames: [ + frame("player", "1:12", "current player"), + frame("controls", "1:12", "current controls") + ] + }) + + await replayBatch({ + revision: "1:12", + token: "reconnect", + targets: [ "player", "controls" ] + }) + await page.waitForFunction( + () => + document.getElementById("player").dataset.solidObjectsRevision === "1:12" && + document.getElementById("controls").dataset.solidObjectsRevision === "1:12", + null, + { timeout: 5000 } + ) + + assert.match(await textOf("player"), /current player/) + assert.match(await textOf("controls"), /current controls/) +}) + +test("a reconnect converges an unbatched component", async () => { + components.set("single", frameHtml("player", "1:12", "current player")) + + await replayComponent({ target: "player", token: "single" }) + await page.waitForFunction( + () => document.getElementById("player").dataset.solidObjectsRevision === "1:12", + null, + { timeout: 5000 } + ) + + assert.match(await textOf("player"), /current player/) +}) + +// A redundant replay must be inert. Morphing a frame that is already current +// would still walk the DOM, discarding anything the page put there since. +test("replaying a revision already applied leaves the frame untouched", async () => { + batches.set("applied", { frames: [ frame("player", "1:12", "current player") ] }) + await replayBatch({ revision: "1:12", token: "applied", targets: [ "player" ] }) + await page.waitForFunction( + () => document.getElementById("player").dataset.solidObjectsRevision === "1:12", + null, + { timeout: 5000 } + ) + await page.evaluate(() => { + const marker = document.createElement("span") + marker.id = "marker" + document.getElementById("player").append(marker) + }) + + batches.set("replay", { frames: [ frame("player", "1:12", "current player") ] }) + await replayBatch({ revision: "1:12", token: "replay", targets: [ "player" ] }) + await page.waitForTimeout(300) + + const marker = await page.evaluate(() => Boolean(document.getElementById("marker"))) + assert.equal(marker, true, "a replay at the applied revision should not morph") +}) + +// A request issued before the drop is still in flight when the reconnect +// arrives. Leaving it running risks its older response landing first and +// flashing stale content, so the reconnect has to cancel it outright. +test("a reconnect cancels the request left in flight by the drop", async () => { + const stalled = gate() + batches.set("before-drop", { + gate: stalled.promise, + frames: [ frame("player", "1:9", "state from before the drop") ] + }) + batches.set("after-reconnect", { + frames: [ frame("player", "1:12", "current player") ] + }) + + await replayBatch({ revision: "1:9", token: "before-drop", targets: [ "player" ] }) + await waitFor( + () => batches.get("before-drop").requested, + "the pre-drop request should reach the server" + ) + await replayBatch({ revision: "1:12", token: "after-reconnect", targets: [ "player" ] }) + + await waitFor( + () => cancelled.has("before-drop"), + "the reconnect should cancel the request from before the drop" + ) + await page.waitForFunction( + () => document.getElementById("player").dataset.solidObjectsRevision === "1:12", + null, + { timeout: 5000 } + ) + stalled.release() + await page.waitForTimeout(300) + + assert.equal(await revisionOf("player"), "1:12") + assert.match(await textOf("player"), /current player/) +}) + +// Destroy and recreate resets the state revision, so the incarnation has to +// decide the ordering. Comparing revisions alone would reject 2:1 after 1:12. +test("a reconnect after a destroy and recreate applies the new incarnation", async () => { + batches.set("first", { frames: [ frame("player", "1:12", "first incarnation") ] }) + await replayBatch({ revision: "1:12", token: "first", targets: [ "player" ] }) + await page.waitForFunction( + () => document.getElementById("player").dataset.solidObjectsRevision === "1:12", + null, + { timeout: 5000 } + ) + + batches.set("second", { frames: [ frame("player", "2:1", "second incarnation") ] }) + await replayBatch({ revision: "2:1", token: "second", targets: [ "player" ] }) + await page.waitForFunction( + () => document.getElementById("player").dataset.solidObjectsRevision === "2:1", + null, + { timeout: 5000 } + ) + + assert.match(await textOf("player"), /second incarnation/) +}) + +test("a reconnect delivers each payload revision once", async () => { + await page.evaluate(() => { + window.payloads = [] + document.addEventListener("solid-objects:payload", (event) => { + window.payloads.push([ event.detail.instanceId, event.detail.revision ]) + }) + }) + + await replayPayload({ name: "playmat_state", revision: "1:12", payload: { turn: 4 } }) + await replayPayload({ name: "playmat_state", revision: "1:12", payload: { turn: 4 } }) + await replayPayload({ name: "playmat_state", revision: "1:13", payload: { turn: 5 } }) + await page.waitForFunction(() => window.payloads.length >= 2, null, { timeout: 5000 }) + await page.waitForTimeout(200) + + const delivered = await page.evaluate(() => window.payloads) + assert.deepEqual(delivered, [ [ 1, 12 ], [ 1, 13 ] ]) +}) + +test("a stale payload replayed after a newer one is dropped", async () => { + await page.evaluate(() => { + window.payloads = [] + document.addEventListener("solid-objects:payload", (event) => { + window.payloads.push(event.detail.payload.turn) + }) + }) + + await replayPayload({ name: "playmat_state", revision: "1:13", payload: { turn: 5 } }) + await page.waitForFunction(() => window.payloads.length === 1, null, { timeout: 5000 }) + await replayPayload({ name: "playmat_state", revision: "1:12", payload: { turn: 4 } }) + await page.waitForTimeout(200) + + assert.deepEqual(await page.evaluate(() => window.payloads), [ 5 ]) +}) + +test("a reconnect burst applies alongside a payload in the same scope", async () => { + await page.evaluate(() => { + window.payloads = [] + document.addEventListener("solid-objects:payload", (event) => { + window.payloads.push(event.detail.revision) + }) + }) + batches.set("burst", { + frames: [ + frame("player", "1:12", "current player"), + frame("controls", "1:12", "current controls") + ] + }) + + await replayBatch({ + revision: "1:12", + token: "burst", + targets: [ "player", "controls" ] + }) + await replayPayload({ name: "playmat_state", revision: "1:12", payload: { turn: 4 } }) + await page.waitForFunction( + () => + window.payloads.length === 1 && + document.getElementById("controls").dataset.solidObjectsRevision === "1:12", + null, + { timeout: 5000 } + ) + + assert.equal(await revisionOf("player"), "1:12") + assert.deepEqual(await page.evaluate(() => window.payloads), [ 12 ]) +}) diff --git a/test/integration/component_batch_test.rb b/test/integration/component_batch_test.rb index 51bb399..52d11f9 100644 --- a/test/integration/component_batch_test.rb +++ b/test/integration/component_batch_test.rb @@ -147,6 +147,67 @@ class BoardActor < SolidObjects::Actor end end + # A dropped connection is the worst moment for request amplification: a + # server restart reconnects every client at once. Reconnect must cost what a + # live invalidation costs. + test "a reconnect batches the components that share a batch" do + subscriptions = subscriptions_for( + player: "playmat", + controls: "playmat", + library: "playmat" + ) + + streams = subscriptions.reconnect_refreshes(advanced_snapshot) + + assert_equal 1, streams.length, + "a reconnect should cost one request per batch, not one per component" + assert_includes streams.first, "solid-objects-batch-refresh" + assert_equal 3, streams.first[/data-targets="([^"]+)"/, 1].split.length + end + + test "a reconnect keeps unbatched components on their own refresh" do + subscriptions = SolidObjects::ComponentSubscriptions.new( + registrations(player: "playmat", controls: "playmat", chat: nil) + ) + + streams = subscriptions.reconnect_refreshes(advanced_snapshot) + + assert_equal 2, streams.length + assert_equal 1, streams.count { |stream| + stream.include?("solid-objects-batch-refresh") + } + end + + test "a reconnect refreshes distinct batches separately" do + subscriptions = SolidObjects::ComponentSubscriptions.new( + registrations(player: "playmat", chat: "sidebar") + ) + + streams = subscriptions.reconnect_refreshes(advanced_snapshot) + + batches = streams.map { |stream| stream[/data-batch="([^"]+)"/, 1] } + assert_equal %w[playmat sidebar], batches.sort + end + + test "a reconnect records what it transmitted for every batched component" do + subscriptions = subscriptions_for(player: "playmat", controls: "playmat") + current = advanced_snapshot + + subscriptions.reconnect_refreshes(current) + replayed = subscriptions.refreshes_for( + invalidation("player", revision: current.revision) + ) + + assert_empty replayed, + "an invalidation already covered by the reconnect must not refresh again" + end + + test "a reconnect leaves current components alone" do + subscriptions = subscriptions_for(player: "playmat", controls: "playmat") + + assert_empty subscriptions.reconnect_refreshes(snapshot) + end + test "the batch url requests every changed component once" do group = registrations(player: "playmat", controls: "playmat") @@ -185,6 +246,15 @@ def registrations(**batches) end end + # Models a client whose registrations were signed before the state moved on, + # which is what a reconnect after a dropped connection looks like. + def advanced_snapshot + current = snapshot + Struct + .new(:instance_id, :revision) + .new(current.instance_id, current.revision + 1) + end + def subscriptions_for(**batches) SolidObjects::ComponentSubscriptions.new(registrations(**batches)) end