From aa641488327276282055cf3f1577ff50ed6723d3 Mon Sep 17 00:00:00 2001 From: OneTooMany Date: Wed, 14 Oct 2026 11:43:15 +0200 Subject: [PATCH 1/3] quest(m1): plan the subscribe-close throw fix and its backport Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 2 + quest/m1/subscribe-close-backport.md | 92 ++++++++++++++++++++++++++++ quest/m1/subscribe-close-throw.md | 64 +++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 quest/m1/subscribe-close-backport.md create mode 100644 quest/m1/subscribe-close-throw.md diff --git a/quest/m1/README.md b/quest/m1/README.md index 0aa9ab6b4c0..a06dd503287 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -75,6 +75,8 @@ above them. - [Quiet latest group test](/quest/m1/quiet-edge-test.md) - pins on `main` that a quiet track's latest group survives rejoin, expiry, and takeover - [FETCH max-delay](/quest/m1/fetch-max-delay.md) - historical readers obey publisher max-age and an optional tighter reader budget, including while holding a group - [Draft-20 FETCH edge cases](/quest/m1/fetch-edge-cases.md) - an exact-final range reports End of Track and a FETCH_OK past the requested group closes the session, on main then release +- [A local close never throws](/quest/m1/subscribe-close-throw.md) - closing a `@moq/net` lite subscription never raises ProtocolViolation for the end of stream it caused, on main +- [Subscribe-close backport](/quest/m1/subscribe-close-backport.md) - the same fix and the `race` loser fix on `release`, so closing a subscription during setup leaves no unhandled rejection in a later 0.17 patch - [Main live-feed check](/quest/m1/fetch-live-feed.md) - tests pin that a fetch-only or parked warm cache never supplies Largest Object on main, fixing it if it does - [Claim answer epochs](/quest/m1/claim-answer-epochs.md) - a claim answers each output by publishing it at its exact path under its own epoch, so a restarted output never reaches a returning reader as the old one's cache - [Untimed decisions](/quest/m1/untimed-decisions.md) - the maintainer decides whether moq-archive keeps refusing untimed tracks, and whether a malformed FETCH object ends its track diff --git a/quest/m1/subscribe-close-backport.md b/quest/m1/subscribe-close-backport.md new file mode 100644 index 00000000000..d98d9d27de5 --- /dev/null +++ b/quest/m1/subscribe-close-backport.md @@ -0,0 +1,92 @@ +# [S] Backport the subscribe-close fixes to 0.4.x + +## Goal + +On `release`, closing a `@moq/net` subscription never leaves an unhandled +rejection and never throws for an end of stream the client caused itself, so +a 0.17 patch after the pending cut (#5420) ships it. The repro below, run +against a `release` build of `@moq/net` and `@moq/signals`, reports +`unhandled=0` over 20 iterations; on 0.4.3 it reports 20. + +## Plan + +[The main fix](/quest/m1/subscribe-close-throw.md) explains the three +defects. `release` has all three. + +Decided 2026-10-14: + +- One PR onto `release` (CONTRIBUTING) that ports two fixes: + - the `#runResponses` fix from the main quest; + - the `raceUntil` loser fix from ca8995280, its `js/signals` hunk and test + only (the rest of #5232 is unrelated). +- It does not hold #5420; it ships in a later 0.17 patch, like the + [FETCH edge cases](/quest/m1/fetch-edge-cases.md). The `@moq/signals` and + `@moq/net` patch bumps happen at that cut. +- `#openSubscribe` keeps sending a SUBSCRIBE for a producer closed during + setup. With both fixes that is a SUBSCRIBE cancelled at once, and `main` + avoids it through its retire logic, so the backport stays minimal. + +Tests: port the main quest's tests. The close-during-setup test fails on +`release` without this PR. + +Repro, with `moq-relay` on this `relay.toml`: + +```toml +[listen] +bind = "127.0.0.1:14443" +tls.generate = ["localhost"] +[web.http] +listen = "127.0.0.1:14443" +[auth] +public = "**" +``` + +and `bun repro.ts http://127.0.0.1:14443/ 20 micro`: + +```ts +import * as Moq from "@moq/net"; +const url = new URL(process.argv[2] ?? "http://127.0.0.1:14443/"); +const iterations = Number(process.argv[3] ?? 20); +const delay = process.argv[4] ?? "0"; +if (!process.env.VERBOSE) console.debug = () => {}; +let unhandled = 0; +process.on("unhandledRejection", (err) => { unhandled++; console.log(`UNHANDLED #${unhandled}: ${err}`); }); +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +const wait = () => delay === "micro" + ? (async () => { for (let i = 0; i < 20; i++) await Promise.resolve(); })() + : sleep(Number(delay)); +const path = Moq.Path.from(`repro/${crypto.randomUUID()}`); + +// Publisher A: one track, one group left open. +const originA = new Moq.Origin.Producer(); +const broadcast = originA.createBroadcast(path); +const track = broadcast.createTrack("room.json"); +const group = track.appendGroup(); +group.writeFrame({ payload: new TextEncoder().encode("snapshot"), timestamp: Moq.Time.Timestamp.now() }); +broadcast.announce(); +const connA = await Moq.Connection.connect({ url, publish: originA.consume() }); + +// Subscriber B. +const originB = new Moq.Origin.Producer(); +const connB = await Moq.Connection.connect({ url, consume: originB }); +for (let i = 0; i < iterations; i++) { + const request = originB.request(path, { announced: true }); + let active = request.active.peek(); + while (!active) { + await new Promise((r) => { const d = request.active.subscribe(() => { d(); r(undefined); }); }); + active = request.active.peek(); + } + const sub = active.track("room.json").subscribe({ priority: 0 }); + await wait(); // close while the subscribe still waits for TRACK_INFO + request.close(); + sub.close(); + await sleep(20); +} +await sleep(200); connB.close(); connA.close(); await sleep(100); +console.log(`delay=${delay} iterations=${iterations} unhandled=${unhandled}`); +process.exit(0); +``` + +## Required + +- [A local close never throws](/quest/m1/subscribe-close-throw.md) - the `main` fix this ports diff --git a/quest/m1/subscribe-close-throw.md b/quest/m1/subscribe-close-throw.md new file mode 100644 index 00000000000..2748e10e7c0 --- /dev/null +++ b/quest/m1/subscribe-close-throw.md @@ -0,0 +1,64 @@ +# [S] A local close never throws for a missing SUBSCRIBE_END + +## Goal + +On `main`, the `@moq/net` lite subscriber no longer raises +`ProtocolViolation: subscribe stream ended without SUBSCRIBE_END` for an end +of stream it caused itself by closing the subscription. A publisher that FINs +without SUBSCRIBE_END on a track the subscriber still holds open still fails +with it. The lite draft states that a subscriber's cancel owes no +SUBSCRIBE_END. + +Non-goals: the release line, which +[the backport](/quest/m1/subscribe-close-backport.md) carries, and skipping +the SUBSCRIBE for a producer closed during setup. + +## Plan + +Found on `@moq/net` 0.4.3 against moq-relay `main`: a peer that closes a +subscription while it still waits for TRACK_INFO gets that ProtocolViolation +as an unhandled rejection (a console error in browsers, a failed test in +Bun's runner, an exit in Node). Three client defects combine: + +1. On `release`, `#openSubscribe` sends SUBSCRIBE for a track producer that + `request.close()` already closed. `main` retires that setup instead. +2. `race` stopped attaching handlers once an entry settled, so the + serve loop's `subscriptionUpdates.then(() => closed)` rejected unobserved. + Fixed on `main` with a test in ca8995280 (#5232); no `@moq/signals` release + carries it yet. +3. `#runResponses` throws when the subscribe stream ends without + SUBSCRIBE_END, including on every ordinary unsubscribe. The serve loop runs + `producer.close(); stream.close()`, and `Stream.close()` cancels the reader, + so the decode sees end of stream. With a FIN alone the relay answers with + its own FIN and no SUBSCRIBE_END, which is right: the draft has a peer echo + a FIN, and `rs/moq-net/src/lite/publisher.rs` treats the requester's FIN as + the normal end. Today `closed.catch` swallows the throw, so it is harmless + only while every caller keeps swallowing it. This quest fixes 3. + +Decided 2026-10-14: + +- Skip the throw when the track producer is already closed: + `entry.end === undefined && entry.track.closed.peek() === undefined`. + Every local close closes the producer before the stream (the serve loop, + `Subscriber.close()`), and a locally closed track owes no end. Chosen over + an explicit cancel flag on the entry, which every close path would have to + remember to set. +- One sentence in `drafts/draft-lcurley-moq-lite.md`, next to the subscribe + stream's SUBSCRIBE_END and FIN text: a subscriber cancels by closing its + side (FIN), the publisher answers with its own FIN, and no SUBSCRIBE_END is + owed. No wire change. + +Tests in `js/net/src/lite/subscriber.test.ts`, with mock streams: + +- An ordinary unsubscribe (subscribe, receive a group, close) whose peer + echoes the FIN without SUBSCRIBE_END ends without a ProtocolViolation. + Assert on the promise, not on a log; it fails without the fix. +- A publisher FIN without SUBSCRIBE_END on a track still open closes the + track with ProtocolViolation. +- Closing the broadcast request after the track subscribe is dequeued but + before TRACK_INFO leaves no unhandled rejection, using the + `unhandledRejection` pattern from `js/net/src/lite/publisher.test.ts`. + +## Related + +- [Backport to 0.4.x](/quest/m1/subscribe-close-backport.md) - carries this fix and the `race` fix onto `release` From 2145cc61960047894421f02b41824289bc0d8150 Mon Sep 17 00:00:00 2001 From: OneTooMany Date: Wed, 14 Oct 2026 11:46:27 +0200 Subject: [PATCH 2/3] quest(m1): pin how the subscribe-close tests observe the fix Co-Authored-By: Claude Opus 5.5 --- quest/m1/subscribe-close-throw.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/quest/m1/subscribe-close-throw.md b/quest/m1/subscribe-close-throw.md index 2748e10e7c0..c347b9b2290 100644 --- a/quest/m1/subscribe-close-throw.md +++ b/quest/m1/subscribe-close-throw.md @@ -52,12 +52,15 @@ Tests in `js/net/src/lite/subscriber.test.ts`, with mock streams: - An ordinary unsubscribe (subscribe, receive a group, close) whose peer echoes the FIN without SUBSCRIBE_END ends without a ProtocolViolation. - Assert on the promise, not on a log; it fails without the fix. + `closed.catch` swallows the rejection today, so the test observes the + responses promise itself through a narrow seam; a track-closure or log + assertion would pass without the fix. - A publisher FIN without SUBSCRIBE_END on a track still open closes the track with ProtocolViolation. - Closing the broadcast request after the track subscribe is dequeued but before TRACK_INFO leaves no unhandled rejection, using the - `unhandledRejection` pattern from `js/net/src/lite/publisher.test.ts`. + `unhandledRejection` pattern from `js/net/src/lite/publisher.test.ts`. The + mock withholds TRACK_INFO until the close, so the ordering is explicit. ## Related From f687d9dc4e888a3b4b7bc362553539ecb6389e07 Mon Sep 17 00:00:00 2001 From: OneTooMany Date: Wed, 14 Oct 2026 11:58:24 +0200 Subject: [PATCH 3/3] quest(m1): make the backport's mock test the gate, not the relay repro Co-Authored-By: Claude Opus 5.5 --- quest/m1/subscribe-close-backport.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/quest/m1/subscribe-close-backport.md b/quest/m1/subscribe-close-backport.md index d98d9d27de5..8239fdac32a 100644 --- a/quest/m1/subscribe-close-backport.md +++ b/quest/m1/subscribe-close-backport.md @@ -26,8 +26,14 @@ Decided 2026-10-14: setup. With both fixes that is a SUBSCRIBE cancelled at once, and `main` avoids it through its retire logic, so the backport stays minimal. -Tests: port the main quest's tests. The close-during-setup test fails on -`release` without this PR. +Tests: port the main quest's tests. The close-during-setup test, which +withholds TRACK_INFO until the close, is the gate and fails on `release` +without this PR. + +The repro below only sees the public API, so it times the close with +microtasks rather than observing the TRACK_INFO read. Run it on the unpatched +`release` build first and confirm 20 of 20, so a 0 on the patched build +means the fix and not a shifted race. Repro, with `moq-relay` on this `relay.toml`: