akiomik / akiomik/nostui

A media subscription that dies is restarted by luck, or in a hot loop

未关闭
#529 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
bug
主要语言
Rust
星标
71
派生
5
平均合并
4 小时 2 分钟
30 天内合并 PR
31

描述

Found while reviewing #528, which is where the property changed. Not a defect in the code that PR removes — an implicit safety net that removal takes with it, and one worth replacing deliberately rather than losing by accident.

## The mechanism

tears marks the declared subscription set dirty on any pass where `update` ran (`updated > 0` → `dirty`, `kernel/pass.rs:198`), and `frame_step` then reconciles, re-admitting everything declared that is not currently running (`kernel/pass.rs:224`, `:280`). What it does *not* do is notice a source ending: `application.rs` says so outright — "a source that merely *finishes* marks nothing dirty on its own" — and restarts it "at the next re-evaluation, whenever that comes".

So a subscription that ends is restarted by the next message from some *other* source. Until #528 nostui had a 16 Hz tick, so "whenever that comes" was "within about 62 ms", always, whether or not anything else was happening. Now it is "whenever anything else happens" — immediate on a feed with traffic, and never on a nostui with none.

Two more details decide how this actually plays out:

- `reconcile` skips a declared subscription whose previous run has not been reflected as finished yet (`kernel/pass.rs:282`), so the *first* re-evaluation after a source ends can miss it. The next message tries again.
- A source whose stream ends by reporting an error supplies that first message itself, which is why the unbuildable case retries rather than stopping.

## Where it bites

**Narrower than this issue first claimed.** Two of the three subscriptions I originally listed turned out not to be reachable, and the corrections are worth keeping so nobody re-derives them:

- **`TerminalEvents` — not affected.** tears wraps crossterm's `EventStream` in a `stream::unfold` that ends when the inner stream yields `None`, which is what made it look exposed. crossterm's `EventStream::poll_next` never returns `Poll::Ready(None)` (`crossterm-0.29.0/src/event/stream.rs:104`): every arm is `Some(Ok(..))`, `Some(Err(..))`, or `Pending`. The keyboard cannot die this way.
- **`NostrEvents` — not reachable from nostui's own code, but not guaranteed either.** `run_subscription_loop` breaks when `notifications.next()` returns `None`. Two things can end that stream. The underlying `BroadcastStream` ends once every sender is dropped — that is, once the `Client` is gone, which is process teardown. But `Client::notifications()` also wraps it in a `take_until` fired by `ClientNotification::Shutdown` (`nostr-sdk-0.45.2/src/client/mod.rs:199`), so a shutdown ends the stream promptly. nostui never calls `Client::shutdown()` — the worker's own shutdown arm calls `disconnect()` — so nothing takes that path today. `runtime.rs` does handle the notification (`notify_subscription_shutdown`, which writes "disconntected"), so the code half-anticipates it. Treat it as the dependency's behaviour rather than an invariant: if that path ever opens, relay traffic stays dead with only the status line to say so.

What is left is **`MediaEvents`**, and it ends two different ways (`src/infrastructure/subscription/media.rs`).

When `MediaSourceBuilder::build()` fails, the stream yields a single `Err` and ends. That `Err` is itself a message, so it marks the set dirty, tears restarts the finished subscription, `build()` fails again, and round it goes: an unbounded build-fail/log loop with no backoff, bounded only by how fast `build()` fails. This one predates #528 — the error message, not the tick, is what drove the re-evaluation — so removing the tick neither caused nor fixed it.

Two things about it are worth stating precisely, because they pull in opposite directions:

- **The retry is not immediate.** `reconcile` skips a declared subscription while its previous run is still registered as running (`tears-0.11.0/src/kernel/pass.rs:282`), and the re-evaluation the `Err` triggers may arrive before the run has been reflected. Lose that race and the restart waits for the next message from anything else, exactly like the silent case below. Win it and the rebuild is immediate, which is what makes the loop a loop.
- **The repaint per iteration is the only thing costing it anything, and it stays.** #528 briefly declined the redraw on the error arm — nothing it does reaches the screen, so the declaration was true — and then kept it. The render is not a designed brake, but it is the sole per-iteration cost in the loop, and how much it actually slows things down was not measured: a `terminal.draw` of an unchanged screen still renders the timeline into a buffer even though the diff flushes almost nothing, and a `build()` that fails for a missing session bus can fail almost instantly. Rather than guess at that ratio and ship the faster spin, the arm keeps its redraw until this issue fits a real bound. A test pins it so the inconsistency with its neighbours is not tidied away.

When the build *succeeds* and the source dies afterwards, nothing is reported at all. On Linux, nowhear spawns its D-Bus task as `let _handle: JoinHandle> = tokio::spawn(...)` and the body opens its signal streams with `await?` (`nowhear-0.5.2/src/platform/linux/provider.rs:405`). If the session bus is unavailable or restarts, the task returns `Err` into a discarded handle, its sender drops, and nostui's stream ends having produced **zero** items. Nothing reaches `update`, so nothing re-evaluates the set. With `nip38.enabled`, "now playing" publishing is then silently dead for the rest of the session: nothing logged, nothing on screen, and — with no other traffic — nothing that will ever restart it.

That last case is the one #528 changed: before it, the tick re-evaluated within 62 ms and the media source came back on its own regardless of what else was happening. It is not permanent — the next relay event or keypress reconciles and restarts it — so the exposure is a nostui that is otherwise completely idle, where there is no next message. An earlier revision of this issue said "silently dead for the rest of the session"; that was wrong, and the correction is the difference between a bug and an inconvenience.

## Why the answer is not "put the timer back"

The tick restarted things by accident, at a fixed rate, whether or not anything needed restarting — that is what #527 removed, and the `MediaEvents` case shows the cost of restarting blindly. What is wanted is recovery expressed as a decision:

- notice that a subscription ended (it has to become a message to be noticed at all),
- restart it on a bounded, backed-off schedule rather than as fast as a timer fires,
- and stop, with something on screen, when it keeps failing.

That overlaps #519 (nostui does not notice when the Nostr worker dies) and #518 (a tab opened while the worker is dead stays on "loading..."): both are about the application not learning that its connection to the worker is gone. This issue is the same shape one layer out, for the subscription itself, and the three probably want one answer.

## Acceptance

- A media source that dies after a successful build is restarted, rather than staying silently dead until unrelated traffic arrives.
- A media source that cannot be built is not rebuilt in a hot loop, and does not write a `log::error!` line per iteration into `nostui.log` while it spins. #528 stopped each round also costing a render, which removed the loop's only outward sign — the log is now the only place it shows.
- Whatever shape that takes generalises: a subscription that ends is noticed as an event, not by a timer that re-evaluates everything on a fixed period.

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。