Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates real-time push from WebSockets to SSE while keeping a temporary WebSocket compatibility path for rolling deploys, updates both frontends to consume SSE, and adds rollout/shutdown configuration for long-lived push connections.
Changes:
- Adds SSE connection, middleware, broker fanout, diagnostics, and
EnablePushconfiguration. - Replaces Svelte/Angular WebSocket clients with fetch/stream-based SSE clients and updates tests.
- Adds Kubernetes drain settings and documents push being disabled until ingress supports SSE.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/Exceptionless.Web/Hubs/SseConnection.cs |
Adds bounded SSE write queue, deduplication, keep-alives, and disposal. |
src/Exceptionless.Web/Hubs/SseConnectionManager.cs |
Manages SSE connections, keep-alives, cleanup, and metrics. |
src/Exceptionless.Web/Hubs/SseMiddleware.cs |
Adds authenticated /api/v2/push SSE endpoint. |
src/Exceptionless.Web/Hubs/WebSocketPushMiddleware.cs |
Adds temporary WebSocket compatibility middleware. |
src/Exceptionless.Web/Hubs/WebSocketConnectionManager.cs |
Reworks WebSocket manager for compatibility fanout and metrics. |
src/Exceptionless.Web/Hubs/MessageBusBroker.cs |
Fans out push messages to SSE and WebSocket managers. |
src/Exceptionless.Web/Hubs/MessageBusBrokerMiddleware.cs |
Removes old WebSocket-only middleware. |
src/Exceptionless.Web/Startup.cs |
Wires SSE and WebSocket push middleware behind EnablePush. |
src/Exceptionless.Web/Program.cs |
Adds host shutdown timeout for drain alignment. |
src/Exceptionless.Web/Bootstrapper.cs |
Registers the SSE connection manager. |
src/Exceptionless.Web/Utility/Handlers/ThrottlingMiddleware.cs |
Renames push throttle exemption to SSE-oriented naming. |
src/Exceptionless.Core/Configuration/AppOptions.cs |
Replaces EnableWebSockets with EnablePush and legacy fallback. |
src/Exceptionless.Core/Bootstrapper.cs |
Updates push-disabled startup warning. |
src/Exceptionless.Core/Utility/AppDiagnostics.cs |
Adds SSE/WebSocket push connection counters. |
src/Exceptionless.Web/ClientApp/src/lib/features/websockets/sse-client.svelte.ts |
Adds Svelte SSE client with reconnect and stream parsing. |
src/Exceptionless.Web/ClientApp/src/lib/features/websockets/sse-client.test.ts |
Adds Svelte SSE client unit tests. |
src/Exceptionless.Web/ClientApp/src/lib/features/websockets/web-socket-client.svelte.ts |
Removes Svelte WebSocket client. |
src/Exceptionless.Web/ClientApp/src/lib/features/websockets/web-socket-client.test.ts |
Removes WebSocket client tests. |
src/Exceptionless.Web/ClientApp/src/routes/(app)/+layout.svelte |
Switches app layout push integration to SSE. |
src/Exceptionless.Web/ClientApp.angular/components/websocket/websocket-service.js |
Rewrites Angular legacy push service to SSE. |
tests/Exceptionless.Tests/Hubs/SseTests.cs |
Adds SSE manager, broker, and deduplication tests. |
tests/Exceptionless.Tests/Hubs/SseIntegrationTests.cs |
Adds HTTP pipeline tests for SSE endpoint behavior. |
tests/Exceptionless.Tests/Hubs/FakeHttpResponse.cs |
Adds fake response helper for SSE tests. |
tests/Exceptionless.Tests/Hubs/WebSocketCompatibilityTests.cs |
Adds WebSocket compatibility and dual-fanout tests. |
tests/Exceptionless.Tests/Hubs/WebSocketTests.cs |
Removes old WebSocket broker tests. |
tests/Exceptionless.Tests/Hubs/WebSocketConnectionManagerTests.cs |
Removes old WebSocket manager tests. |
tests/Exceptionless.Tests/Hubs/TestWebSocket.cs |
Updates test WebSocket encoding and close count layout. |
tests/Exceptionless.Tests/appsettings.yml |
Enables push for test host configuration. |
tests/http/push.http |
Adds manual SSE smoke requests for localhost. |
k8s/exceptionless/templates/api.yaml |
Adds drain lifecycle settings and disables push pending ingress migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
09d643e to
7398308
Compare
|
Addressed the outstanding review feedback in the latest pushes: restored the Angular legacy client to the existing WebSocket path with only a deprecation note, tightened SSE/WebSocket lifecycle cleanup, fixed the SSE queue and keepalive backpressure edge case, and stopped the Svelte client from retrying when push is disabled.\n\nLocal validation is green (, , , , and Using launch settings from /Users/blake/Projects/Exceptionless/copilot-worktrees/Exceptionless/niemyjski-stunning-robot/tests/Exceptionless.Tests/Properties/launchSettings.json... [+94/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(5s) [+239/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(8s) [+338/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(11s) [+483/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(14s) [+506/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(17s) [+569/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(20s) [+684/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(23s) [+743/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(26s) [+835/x0/?0] Exceptionless.Tests.dll (net10.0|arm64)(29s) skipped Exceptionless.Tests.Pipeline.EventPipelineTests.GeneratePerformanceDataAsync (0ms) [+1042/x0/?1] Exceptionless.Tests.dll (net10.0|arm64)(35s) [+1151/x0/?1] Exceptionless.Tests.dll (net10.0|arm64)(38s) skipped Exceptionless.Tests.Repositories.EventRepositoryTests.GetAsyncPerformanceAsync (0ms) [+1277/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(44s) [+1424/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(47s) [+1567/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(50s) [+1650/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(53s) [+1792/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(56s) [+1864/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(59s) [+1873/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(1m 02s) [+1873/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(1m 05s) [+1874/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(1m 08s) [+1875/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(1m 11s) [+1876/x0/?3] Exceptionless.Tests.dll (net10.0|arm64)(1m 14s) /Users/blake/Projects/Exceptionless/copilot-worktrees/Exceptionless/niemyjski-stunning-robot/tests/Exceptionless.Tests/bin/Debug/net10.0/Exceptionless.Tests.dll (net10.0|arm64) passed (1m 14s 985ms) Test run summary: Passed! |
ejsmith
left a comment
There was a problem hiding this comment.
The steady-state SSE fanout design should not require sticky sessions: each long-lived stream remains local to the replica that accepted it, Redis pub/sub delivers notifications to every API replica, and only the replica owning a given local connection writes to it. Reconnects can land on any replica.
However, the shared connection metadata currently has two scale-out failure modes that should be addressed before enabling push in production:
- Authentication-token revocation can be consumed destructively by a non-owning replica, leaving the owning replica's stream connected.
- Redis connection reservations have no lease or expiry, so an ungraceful pod/node failure can leave permanent entries that eventually cause users with no live streams to receive 429 responses.
The current tests use a single host/manager and cannot expose these cases. Please add a two-replica test with shared connection state/message bus covering cross-replica delivery and token revocation, plus stale-reservation behavior after simulated replica loss.
|
Follow-up feedback audit complete at
GitHub still reports the historical |
5ce74d8 to
06f8646
Compare
Moves the root Svelte application's real-time updates to authenticated SSE at
/api/v2/push, retaining server WebSocket compatibility for cached clients and existing snake_case payloads. Current main (893d79b1f) is integrated; legacy Angular sources and the replaced Svelte WebSocket client remain removed. SSE uses the configured application API origin independently of telemetry.EnableWebSocketsalias, HTTP examples and OpenAPI documentation remain in scope.Verification at exact head
780c369b39866634cf1f8f6150e44cd807f394c2: Build run 37166410967 passed. Actual results: 3,202 API tests passed / 3 existing skips / 0 failures, including credential/queue canaries, Redis atomic leases, all seven SSE integration cases and all four OpenAPI snapshots; 997 frontend tests passed in 122 files using Vitest 5.0.3 / Vite 8.3.2; all 121 browser tests passed with zero retries. Both shard coverage gates, lint/Svelte check/build, website and container builds passed.Local focused component/query checks passed 10/10; isolated Chromium harness checks passed 4/4 without retries; validation and production build passed. Synthetic credential, queue, navigation and mixed-case regressions reproduced before their fixes. The combined HTTP/2 proxy reproduction released all streams in 24 abort/reload cycles. Independent final scoped source review found no remaining findings.
The PR remains draft. Local service-backed journeys were not rerun while Docker restart approval is pending; hosted CI supplies the runtime evidence above. Ingress streaming validation remains a rollout gate and the chart keeps push disabled. No merge or deployment is included.