Skip to content

Keep the bunker serving through a lagged notification stream - #1074

Merged
kwsantiago merged 1 commit into
mainfrom
a-lagged-bunker-keeps-serving
Oct 18, 2026
Merged

kwsantiago merged 1 commit into
mainfrom
a-lagged-bunker-keeps-serving

Conversation

@kwsantiago

Copy link
Copy Markdown
Contributor

A NIP-46 bunker whose relay notifications fell behind stopped answering and reported a clean, intentional stop, with nothing logged.

Server::run drove requests through Client::handle_notifications, which in nostr-relay-pool 0.44.3 loops on while let Ok(notification) = notifications.recv().await over a broadcast channel of 4096. A receiver that falls more than that behind gets Err(Lagged), and that loop treats it exactly as it treats a shutdown: it ends and returns Ok(()). Reproduced: the dependency's loop on a lagged receiver handled none of the notifications still held and never reached the shutdown.

It is reachable rather than theoretical because requests are handled one at a time, and an approval holds the loop for up to a minute on every surface since #1068. A busy relay can queue 4096 notifications in that time.

The loop is now driven directly from the receiver:

  • A lag logs how many notifications were skipped and keeps serving. The receiver resumes from the oldest one still held, and clients whose requests were dropped time out and retry. Restarting instead would lose the queued ones as well.
  • A Shutdown notification is a clean stop, as before.
  • A stream that closes without a shutdown is now an error rather than Ok(()), so a caller can tell it from a stop it asked for. On mobile that reports the bunker as errored instead of stopped.

The loop is a separate function taking the receiver, so a test can lag it deterministically with a small channel. Tests cover a lagged receiver serving on until the real shutdown and handling what it still held, a stream closing without a shutdown being reported as such, and a shutdown being a clean stop. Making the lag end the loop again fails the first.

Not changed: requests are still handled one at a time, so a slow approval still delays later ones. Handling them concurrently would race the rate limiters and the per-app permission counters, which rely on that ordering today.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 09a4544f-a179-4525-b11c-5a256c8f694a
📥 Commits

Reviewing files that changed from the base of the PR and between 451bbb7 and 3abdf36.

📒 Files selected for processing: 1
  • keep-nip46/src/server.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kwsantiago
kwsantiago merged commit e2d785c into main Oct 18, 2026
13 checks passed
@kwsantiago
kwsantiago deleted the a-lagged-bunker-keeps-serving branch October 18, 2026 17:17
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.

1 participant