Repository navigation
relay: Account::refusal()'s wildcard would swallow a third refusal cause #993
Description
Fixed in #1026 (open, closes this issue on merge). Read this section, not the original report below it: the report was written on 2026-10-25 against the pre-fix tree, and three of its claims did not survive measurement.
The corrected picture
- The type is
FrameSink, notAccount— there is nostruct Accountdoing this in the crate. - There are seven reader sites, not two, and the issue's line numbers moved since relay: the 8 MiB frame bound is per socket and sockets are unlimited #959 (
relay.rs:391isrelay.rs:534at the base48f1e01;subscribe.rs:283-287issubscribe.rs:285). - Only the decode was silent.
sink.rs:164's_ => Nonewas the defect. Both readers the report names were already cause-agnostic and correct —is_some()means "any refusal", andif let Some(r)fed an exhaustivematch. The report's premise, "two readers take that answer as nothing to do", was wrong: it was one arm, and the two readers inherited a bug they never had. - The full table of all seven sites, with what each did, is in this comment.
What the fix does
The decode matches the causes it knows, and the connection loop's arm has no wildcard and no irrefutable binding — it names its variants:
refusal @ (Refusal::QueueFull | Refusal::BudgetExhausted) =>
close_refused gained a Refusal::None => {} arm — "never refused, nothing to close" — rather than a catch-all.
Adding a fourth cause is now an E0004 at four sites, measured on a scratch copy at the fixed head: word, the connection loop (relay.rs:534:19), the read's guard, and close_refused. The two send latch checks remain equality comparisons, so a new cause compiles clean there; that is deliberate and argued — a non-latching cause is already closed by the loop's close_refused, which is how the budget cause works today (ADR 0008) — so send's early return saves an encoding, not the close.
Original report (2026-10-25), kept for the record
Account::refusal() (crates/nostream-relay/src/sink.rs:163-169) ends in _ => None, and Refusal has exactly two variants today, so the arm is unreachable. The problem is what happens when a third cause arrives:
Refusalgains a variant →refuse()andclose_refused()are compiler-forced to handle it (they match exhaustively);Account::refusal()is not —_ => Noneswallows it silently, and now two readers take that answer as "nothing to do":relay.rs:391, the connection loop's top-of-iteration check, so the socket is never closed for the new cause;subscribe.rs:283-287, the read's refusal guard, so the read neither stops nor parks and keeps encoding-and-discarding at the cost measured in relay: the 8 MiB frame bound is per socket and sockets are unlimited #959 (≈450× a latched send).
So the next refusal cause added to this module would be half implemented in a way no test catches: a compile-clean, silently wrong socket.
What it needs
Replace the wildcard with an exhaustive match over Refusal so the compiler forces every reader to decide. If a None case must exist for a state that is genuinely "no refusal", model it as a third variant (None/Clear) rather than as a catch-all — that is the difference between the compiler asking and the reader remembering.
Found by the architecture lens in #959's review; left out of that PR because it is latent and the PR was already bounding two things. No behaviour changes today.
Activity
fix(relay): replace Account::refusal wildcard with exhaustive Refusal…
avantikonde commentedon Nov 1, 2026
I have implemented the fix for this according to the specification outlined above on branch:
https://github.com/avantikonde/nostream/tree/fix/refusal-exhaustive-match
Summary of changes:
Refusal::Clearrather than relying on a catch-all wildcard.Account::refusalandFrameSink::refusalto returnRefusaldirectly, panicking on unknown codes.Refusalacross readers (run_connectioninrelay.rs,fetch_and_sendinsubscribe.rs, andFrameSink::sendinsink.rs).docs/rewrite/decisions/0008-frame-memory-budget.mdand included.changeset.PR creation on the repository is currently restricted to collaborators. Could you please grant me collaborator access so I can open the pull request?
chappie-daemon commentedon Nov 1, 2026
Closed by #1026. The reader set below is the one this issue needed and did not have: it named two sites, and there are seven. Every line is at
48f1e01, the base the job was cut from.48f1e01crates/nostream-relay/src/sink.rs:164match … { QUEUE_FULL => …, BUDGET_EXHAUSTED => …, _ => None }. The only silent site, and the only one the issue's framing understated.sink.rs:265send's latch,== Some(Refusal::QueueFull)— an equality test, not a matchsink.rs:277sink.rs:354FrameSink::refusal(), the passthroughrelay.rs:534if let Some(refusal) = sink.refusal(), feedingclose_refusedrelay.rs:655close_refused'smatch refusal, already exhaustivehandlers/subscribe.rs:285.is_some()Corrections to the issue's own text, since a future reader will start here:
relay.rs:391isrelay.rs:534at48f1e01;subscribe.rs:283-287issubscribe.rs:285. The issue's numbers were taken during relay: the 8 MiB frame bound is per socket and sockets are unlimited #959's review and the file moved.FrameSink, notSink— there is nostruct Sinkin the crate.is_some()means "any refusal", andif let Some(r)feeds an exhaustivematch. The defect was one arm, not two readers — with one exception the issue could not have known: the connection loop'sif letbecame amatchin refactor(relay): decode the refusal word exhaustively #1026 and the first attempt used an irrefutable binding, which the review caught. It now names its two variants, so a fourth cause is anE0004there.Measured, not asserted: a program carrying the decode verbatim returned
None/is_some()=falsefor word3— indistinguishable from no refusal — while the same snippet with a third variant added failed the encode witherror[E0004]: non-exhaustive patterns.Also measured: adding a fourth variant to the merged shape produces
E0004at four sites —word(), the connection loop (relay.rs:534:19), the read's guard, andclose_refused— and not atfrom_word, which is total and traps on a byte it was not given. That last part is stated in the type's doc rather than implied.Two follow-ups filed from the review: #1027 (
sink.rs:36-38credits the encode/decode pair for a consistency only the pin test holds) and #1028 (the pin test's failure message calls the bytes a wire contract they are not).refactor(relay): decode the refusal word exhaustively (#1026)
chappie-daemon commentedon Nov 1, 2026
Fixed by #1026, merged as
beaacde6intorewrite/main.Closed by hand rather than by the keyword:
Closes #993only auto-closes when the pull request targets the repository's default branch, andcameri/nostream's default ismainwhile this PR (like everything on the rewrite) targetsrewrite/main. The link in the PR body therefore did nothing here. Every issue fixed on the rewrite branch needs closing by hand — or the keyword needs a companion line saying so.