Keep the faults dashboard in place while the fault list refreshes - #100
Open
bburda wants to merge 9 commits into
Open
Keep the faults dashboard in place while the fault list refreshes#100bburda wants to merge 9 commits into
bburda wants to merge 9 commits into
Conversation
An empty fault list was indistinguishable from a list that had never loaded: fetchFaults derived "initial load" from faults.length === 0, so every later fetch re-entered the loading state and the dashboard swapped itself for its first-load skeleton. The list now carries a faultsLoaded flag, set once an answer arrives - including an error answer, so a retry does not bring the skeleton back either. Refreshing is also shared now. The dashboard and the sidebar badge each ran their own timer, their own mount fetch and, in the badge, their own visibilitychange listener, so a gateway serving both views answered two requests per refresh. useFaultPolling keeps one timer, one listener and one initial fetch for however many fault views are mounted, and drops the timer while the SSE fault stream is delivering updates.
Three things could put an outdated fault list on screen. A refresh that outlived its session wrote the previous gateway's faults into the next one, so disconnect now clears the list and an answer whose client is gone is dropped. An answer that the fault stream had already overtaken replaced the newer stream state, so a refresh that finds the list rewritten under it keeps out of the way. And the "nothing changed" comparison only looked at code, status and severity, so a fault that moved to another entity or changed its description was thrown away - it now covers every field a row shows, the entity included, which is what the clear action acts on. Views also share the request itself: a refresh already on the wire is reused instead of duplicated, and clearing a fault or pressing refresh forces its own read so it cannot be answered by a request older than the action.
Sharing one refresh went too far: only the first view read on mount, and the sidebar badge is mounted for the whole session, so opening the dashboard showed the list as it stood when the session began. With the fault stream up there is no timer either, so nothing corrected it. Every view now reads when it opens, and again when the connection is replaced or the stream appears or dies - the requests that coincide still collapse into one. Connecting to another gateway also drops the previous one's faults instead of leaving rows whose clear button would address the new gateway. Two ways the list could stop refreshing entirely are gone. A request that never came back held the shared refresh for the life of the tab, so every later refresh waited behind it and the page kept claiming no faults; it now times out, and leaving the session releases it. A fault stream that ended without an error left the client believing it was still delivering, and the timer stayed off for good; the stream ending now hands refreshing back to polling however it ended. Clearing a fault re-read the list twice, once in the store and once in the dashboard.
…onest
A gateway that cannot answer for faults - no fault manager running, so
`/faults` is a 503 - produced the same screen as a system with nothing
wrong: "No faults detected", "System is operating normally". The failure
is now a state of its own, carrying the gateway's own words for it
("Failed to get faults: ListFaults service not available") and a way to
try again, and the header says the fault status is unknown rather than
counting zero. A list that is on screen but could not be refreshed says so
above itself. The error toast is left for refreshes the user asked for:
the page carries the failure now, and one toast per retry stacked up.
Auto-refresh means what it says. The dashboard's switch only ever governed
the dashboard's own timer, while the sidebar badge ran an unconditional one
into the same shared list, so turning it off changed nothing. Switching it
off now stops refreshing for every view of that list.
A connected fault stream is not proof that anything comes down it: an
aggregating gateway answers the subscription and then fans nothing out to
its peers, which from the browser is indistinguishable from a quiet system.
A slow check every 30 seconds runs alongside the stream so the page cannot
sit unchanged for a whole session.
One fault reached through two peers was listed twice and counted twice. A
fault is its code on its entity, so the repeats fold into one row.
…hecked Folding a fault two peers both reported kept whichever came first, so an older record could bury the live one: a peer answering CLEARED before another answered CONFIRMED hid a fault that was still up. The revision that survives is now the one that describes the fault now - still raised over already gone, and at equal standing the higher occurrence count. Reads answer out of order too. A forced refresh starts while a background one is still out, and the older answer could set the list, or set the reason for a read that had since succeeded. Only the newest read writes. The stream merged events by code and entity id while the list is keyed by entity type as well, so an event for a component fault could take out the app fault of the same name. Both use one key now, reached through an action the tests can drive. Two places still spoke for a gateway that had not answered: the green "All Clear" badge stood next to "Fault list unavailable", and the sidebar count kept its last number with nothing to say the reading was stale - it now shows that the fault status is unknown. A refresh triggered by the tab coming back also restarts the timer, instead of leaving a tick to fire moments behind it.
A refresh the user pressed could fail in silence: the rule that keeps an older answer from undoing a newer one compared against the newest read STARTED, so a background read beginning right after the button was pressed made the button's own failure unreportable - no toast, no reason on the page, just a spinner that stopped. An answer is now kept unless a later one has already been applied. The 15 s deadline the page named was never the one in force: the generated client aborts a fault read at 10 s of its own, so that controller was dead code and the number on screen was wrong. The client's deadline stands on its own and the page no longer invents a figure for it. Clearing a fault that the gateway says is already gone left the row on screen with a button that does nothing, because the list is only re-read when the delete succeeds. It is re-read either way now. Auto-refresh belongs to the list, not to the dashboard component: leaving the page reset the switch and quietly resumed refreshing, and a tab regaining focus refreshed a list the user had deliberately frozen. The sidebar badge kept its own count while a refresh was failing and then threw that count away for a question mark - it now shows the number it has, marked as unchecked, and the question mark only when it knows nothing. Rows are ordered worst-first, then most recent, then by identity, so an answer arriving in a different order cannot move a row out from under the pointer on its way to Clear.
A failed read of the whole list left the page saying the fault status was unknown for as long as it took the timer to come round, even while the fault stream was delivering current events - and with refreshing switched off, until someone pressed a button. An event now buys one read: it is proof the gateway answers, which is what the failed read left in doubt. It is not proof the list is complete, so the failure is cleared by the read that succeeds, never by the event itself, and a list the user paused stays paused. One read, not one per event: while the reason for the failure is unchanged a burst of events costs a single attempt, and the timer is what keeps trying. A different reason is a change, and buys its own attempt. Connecting to another gateway also drops the previous one's fault stream there and then. It used to stay open until this connection got as far as subscribing, and its events were written into the list of a gateway that never reported them. Both channels that produce faults run through one transform, and the shared list is keyed on the entity type it fills in, so they have to agree about it. That is written down where the value is decided, with a test that fails if the two channels ever stop matching.
mfaferek93
reviewed
Aug 31, 2026
| const result = transformFaultsResponse(faultsData); | ||
| // The fault stream writes this same list. If it did so while this | ||
| // request was in flight, its state is the newer of the two. | ||
| if (get().faults !== currentFaults) { |
Contributor
There was a problem hiding this comment.
If a stream event lands while the first read of a connection is in flight, this drops the entire baseline: the stream only carries deltas, so every fault that existed before the subscription stays hidden until the next poll, and with the stream up that is 30s away. The overtaken-answer test shows it: FROM_THE_POLL is on the gateway and missing from the final list. Discard is right for later refreshes, but on the initial load I would schedule one more read here instead of returning.
| visibilityListener = refreshAndRestartTimer; | ||
| document.addEventListener('visibilitychange', visibilityListener); | ||
| } | ||
| refreshFaults(); |
Contributor
There was a problem hiding this comment.
This mount read also runs with auto-refresh switched off, so reopening the dashboard moves a list the user froze. The faultsAutoRefresh comment in store.ts promises the opposite, and the navigation test only pins the switch state, not the request. Skipping it when faultsAutoRefresh is false and faultsLoaded is true keeps the freeze honest; the Refresh button covers the rest.
| if (byStatus !== 0) return byStatus > 0 ? b : a; | ||
| const occurrences = (f: Fault) => | ||
| typeof f.parameters?.occurrence_count === 'number' ? f.parameters.occurrence_count : 0; | ||
| return occurrences(b) > occurrences(a) ? b : a; |
Contributor
There was a problem hiding this comment.
With equal status and no occurrence_count on either record, the first-listed peer wins, and its message/timestamp may be the older reading. A timestamp tiebreak fixes it: return b.timestamp > a.timestamp ? b : a;
…he later reading Three things the review asked for. Dropping an answer the stream had overtaken was right for a refresh and wrong for the first read of a connection: the stream carries what happens next, not what was already raised, so a single event landing during that read hid every fault that predated the subscription until the next one. The first read now buys one more read instead, once, and stays unloaded until it lands. The read a view does when it opens ran even with refreshing switched off, so reopening the dashboard moved a list the user had frozen. It is skipped now unless nothing has ever loaded, where an empty page would be the alternative. Of two records for one fault with the same status and occurrence count, the first listed won and could be the older reading. The peer that saw it last wins, and where neither says, the later start does. Two more from the same pass. Faults were grouped by entity name alone, so an app and the component it runs on landed in one group under one of their headings. And "All Clear" spoke for the whole system while filtered rows sat below it, which it cannot do.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The faults dashboard swapped itself for its first-load skeleton every time the fault list refreshed while that list was empty, so the page blinked.
fetchFaultstreatedfaults.length === 0as "not loaded yet", which is also what a gateway with no faults returns. AfaultsLoadedflag separates the two. Following that through the rest of what the page does with refreshing is where the other two reports live.Refreshing is shared and honest. The dashboard and the sidebar badge each owned a timer, a mount fetch and (in the badge) a
visibilitychangelistener, so a gateway serving both answered twoGET /faultsper refresh. One timer, one listener and one request now serve every mounted view, and every view still reads when it opens - the badge is mounted for the whole session, so the dashboard is always a later view and would otherwise show the list as it stood when the session began. Auto-refresh belongs to the list rather than to the dashboard component: it governs every view, survives leaving the page, and a tab regaining focus does not refresh a list the user deliberately froze.A connected fault stream is not proof that anything comes down it. An aggregating gateway answers the subscription and then fans nothing out to its peers, which from the browser is indistinguishable from a quiet system, so the page sat unchanged with the switch reading on. A 30 second check now runs alongside the stream. A stream that ended without an error also used to leave the client believing it was still delivering, with the timer off for good; the stream ending hands refreshing back to polling however it ended.
A gateway that cannot answer for faults no longer looks like a healthy one. With no fault manager running,
/faultsis a 503 and the page said "No faults detected" and "System is operating normally", green "All Clear" badge included. The failure is a state of its own now, carrying the gateway's own words for it and a way to try again; a list on screen that could not be refreshed says so above itself; and the sidebar keeps the count it has, marked as unchecked, rather than throwing it away or presenting it as verified.One fault reached through two peers was listed twice and counted twice. A fault is its code on its entity, so the repeats fold into one row - and the row that survives is the one that describes the fault now, since peers answer at their own pace and an older
CLEAREDmust not bury a liveCONFIRMED.The rest is what a shared refresh has to survive, each with a test: an answer that outlived its session, or that a later read has already answered for, is dropped - but a refresh the user pressed still reports its own failure; a read belongs to the connection that started it, so switching gateways is not answered by the previous one's request; the stream and the list are merged on one key; clearing a fault re-reads the list once, and re-reads it even when the delete failed, so no ghost row is left; the "nothing changed" comparison covers every field a row shows; and rows are ordered worst-first, then most recent, then by identity, so an answer arriving in a different order cannot move a row out from under the pointer.
Issue
Type
Testing
e2e/faults-refresh.spec.tsdrives the real UI against the containerised gateway: the skeleton never returns after a refresh (watched with aMutationObserver, since a one-frame flash slips past a locator poll), opening the dashboard reads the list, a gateway answering 503 produces the unavailable state and keeps it across retries with nothing on screen claiming the system is fine, and the fallback poll asks once per interval with both views mounted. Each was run against the code it guards and fails there.src/lib/store-faults.test.ts,src/lib/store-connect.test.ts,src/lib/transforms.test.tsandsrc/components/FaultsDashboard.polling.test.tsxcover the rest, including the cases that are hard to reach in a browser: a read that never answers, a stream that closes cleanly, an answer overtaken by the stream, a connection replaced mid-read, a failed clear, a hidden tab, the switch surviving navigation, and the two controls that must not change - the skeleton still shows for the very first load, and the 5 second poll stays off while the stream delivers.npm run lint,npm run typecheck,npm run build,npm run format:checkandnpx vitest run(725 tests) are green.Checklist
npm run lint)npm run build)