Fix SSE events running outside the Angular zone - #197
Conversation
LogicalGuy77
commented
Aug 5, 2026
LogicalGuy77
commented
- Clicking on an inference service, didn't show the details page. It was only visible once we reloaded it
Signed-off-by: Harshit Nayan <harshitacademia@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
frontend/src/app/services/sse.service.ts:75
- The primary regression fix is untested: the service test only updates constructor setup and still exercises URL encoding. Add a test that invokes the mocked EventSource callback outside Angular's zone and verifies that subscriber delivery occurs through
NgZone.run; cover the error callbacks as well so a later refactor cannot silently restore the stale-view behavior.
this.zone.run(() => observer.next(data));
frontend/src/app/services/sse.service.ts:16
zoneis ambiguous and less expressive than the injected dependency's role. Rename this field toangularZoneand update its references so readers can distinguish Angular execution context from other meanings of “zone.”
constructor(private zone: NgZone) {}
Rename the injected NgZone field to angularZone for clarity, and add tests that fire the mocked EventSource message and error callbacks from outside the Angular zone, asserting subscribers receive them inside the zone via NgZone.isInAngularZone(). Covers parse errors, connection errors, and reconnect exhaustion so removing the NgZone.run wrapper would fail the suite. Signed-off-by: Harshit Nayan <harshitacademia@gmail.com>
|
@LogicalGuy77 we have fixed similar issue in #194 , did you confirmed that issue still exists by reproducing it against current |
|
yes, I tested it against the current master cc @danish9039 |
|
#194 fixed the most visible instance of that symptom at the navigation layer: in Dashboard-connected mode it bypasses Angular routing and performs a full page load of the details URL, which reboots the application and guarantees a fresh render. That works and is still compatible with this change, but it only covers the row-click path. Any other interaction on DOM rendered from an SSE event like button clicks, menu actions, future features has the same latent problem, and each would need its own workaround. This PR fixes the defect at its source: SSE emissions are re-entered into the Angular zone via NgZone.run(), so change detection runs after every SSE-driven render and event listeners are registered inside the zone. Once this merges, the Dashboard-connected page-load path added in #194 should no longer be necessary, since the stale-render condition it worked around is fixed at the source. |
|
Thank you, Please raise a follow up PR then |
