Skip to content

Reconnect notification streams after broadcast lag to recover missed updates - #1080

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
Floating-Y:fix/notification-sse-lag
Oct 5, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
Floating-Y:fix/notification-sse-lag

Conversation

@Floating-Y

Copy link
Copy Markdown
Contributor

Why

The shared broadcast channel can evict a knowledge base's only notification or the only alert. Continuing after RecvError::Lagged leaves the SSE connection open while the remaining events are filtered out, so the client's reconnect recovery never runs for that loss.

Follow-up to #1036 and #1053, which added reconnect recovery and consolidated notification subscriptions but did not cover broadcast overflow.

What changes

  • End both knowledge-base and standalone alert SSE responses on lag so native EventSource reconnects and invokes the existing recovery callback. KB recovery refreshes current-KB queries and global alerts even when no further business event arrives.
  • Extract a private SSE constructor in each route module so HTTP regressions exercise the production streams.
  • Extend existing frontend recovery tests for current-KB queries, alert badge/list refresh, deduplication, and late callbacks after cleanup.

Authentication, filtering, alert payloads, keep-alives, event coalescing, and hidden-page connection handling retain their existing behavior. Chat-generation streams are untouched; no production frontend or dependency changes are needed.

How it was checked

  • Real HTTP regressions use the 256-entry channel: one relevant event followed by 257 unrelated events. Both tests time out with the old continue branches and pass with the fix, returning HTTP 200 SSE responses that finish with empty bodies. Coverage includes a lost KB document, a lost global alert on the KB stream, and a lost standalone alert.
  • Chrome 154 with native EventSource, production React hooks, and real QueryObservers: all three overflow scenarios changed from one open connection and no refetch to open → error → open and exactly one recovery refetch, without another business event. Other-KB and unrelated queries stayed unchanged; initial connection and cleanup caused no extra refetch. This used a local HTTP fixture around the production SSE constructors; a full deployed UI/authentication walkthrough was not performed.
  • cargo fmt --all --check and cargo clippy --workspace --all-targets -- -D warnings passed.
  • cargo test --workspace: 1,432 passed, 6 existing opt-in tests ignored. Tests used a dedicated migrated PostgreSQL database with UTOPIA_TEST_REQUIRE_DB=1 and UTOPIA_TEST_REQUIRE_PDFTOTEXT=1. The first run hit local proxy failures; the complete successful rerun set process-local NO_PROXY=127.0.0.1,localhost,::1.
  • pnpm install --frozen-lockfile, pnpm test (228 tests), and pnpm build passed.

An additional cargo build --workspace attempt failed when Cargo tried to replace the existing, running target/debug/utopia-server.exe on Windows (access denied). That service was left running; the full non-test build is not reported as passing.

Before review

  • Every commit is signed off (git commit -s)
  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings and cargo test --workspace pass
  • For changes under web/: pnpm build and pnpm test pass

…updates

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Floating-Y. Right: after a lag the stream stayed open, the lost event may have been the only one this listener would ever get, and nothing told the page to catch up. Ending the response reuses the recovery the page already has, so there is no new event to teach the client, and it costs two changed lines outside the tests. The filtering and alert payloads are pinned by the second test as they were. Landing it.

@WaylandYang
WaylandYang merged commit 229b846 into deeplethe:dev Oct 5, 2026
6 checks passed
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.

2 participants