Skip to content

Commit 89fc7d1

Browse files
committed
fix: let a fault refresh see the state it is answering
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.
1 parent f6f6d3d commit 89fc7d1

4 files changed

Lines changed: 274 additions & 48 deletions

File tree

src/components/FaultsDashboard.polling.test.tsx

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
*/
2121

2222
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
23-
import { render, act } from '@testing-library/react';
23+
import { render, act, type RenderResult } from '@testing-library/react';
24+
import type { ReactElement } from 'react';
2425
import { FaultsDashboard, FaultsCountBadge } from './FaultsDashboard';
2526
import { useAppStore } from '@/lib/store';
2627

@@ -61,20 +62,28 @@ async function framesDuring(container: HTMLElement, run: () => Promise<void>): P
6162
return frames;
6263
}
6364

65+
/** Renders and lets the refresh the mount starts finish, so nothing settles outside act. */
66+
async function mount(ui: ReactElement): Promise<RenderResult> {
67+
let result!: RenderResult;
68+
await act(async () => {
69+
result = render(ui);
70+
await vi.advanceTimersByTimeAsync(0);
71+
});
72+
return result;
73+
}
74+
75+
/** Lets everything already scheduled run: timers due now, and the promises they start. */
6476
async function settle() {
6577
await act(async () => {
66-
await Promise.resolve();
67-
await Promise.resolve();
68-
await Promise.resolve();
78+
await vi.advanceTimersByTimeAsync(0);
6979
});
7080
}
7181

82+
/** Moves time forward, letting each tick's request settle before the next one fires. */
7283
async function advance(ms: number) {
7384
await act(async () => {
74-
vi.advanceTimersByTime(ms);
75-
await Promise.resolve();
85+
await vi.advanceTimersByTimeAsync(ms);
7686
});
77-
await settle();
7887
}
7988

8089
function connect(client: ReturnType<typeof clientReturning>, sseActive: boolean) {
@@ -93,15 +102,19 @@ beforeEach(() => {
93102
});
94103

95104
afterEach(() => {
105+
// Wrapped: this hook runs while the views are still mounted (Testing Library's
106+
// own cleanup is registered earlier and so runs after this one), and an
107+
// unwrapped store write would land on them outside act.
108+
act(() => {
109+
useAppStore.setState({ isConnected: false, client: null, faults: [], faultStreamCleanup: null } as never);
110+
});
96111
vi.useRealTimers();
97-
useAppStore.setState({ isConnected: false, client: null, faults: [], faultStreamCleanup: null } as never);
98112
});
99113

100114
describe('FaultsDashboard refresh behaviour', () => {
101115
it('never falls back to the first-load skeleton once the empty list has loaded', async () => {
102116
connect(clientReturning([]), false);
103-
const { container } = render(<FaultsDashboard />);
104-
await settle();
117+
const { container } = await mount(<FaultsDashboard />);
105118

106119
const frames = await framesDuring(container, () => advance(POLL_INTERVAL_MS));
107120

@@ -110,13 +123,12 @@ describe('FaultsDashboard refresh behaviour', () => {
110123

111124
it('never falls back to the skeleton when the tab regains focus', async () => {
112125
connect(clientReturning([]), true);
113-
const { container } = render(
126+
const { container } = await mount(
114127
<>
115128
<FaultsCountBadge />
116129
<FaultsDashboard />
117130
</>
118131
);
119-
await settle();
120132

121133
const frames = await framesDuring(container, async () => {
122134
await act(async () => {
@@ -142,13 +154,12 @@ describe('FaultsDashboard refresh behaviour', () => {
142154
it('asks the gateway once per interval even with the badge and the dashboard mounted', async () => {
143155
const client = clientReturning([RAW_FAULT]);
144156
connect(client, false);
145-
render(
157+
await mount(
146158
<>
147159
<FaultsCountBadge />
148160
<FaultsDashboard />
149161
</>
150162
);
151-
await settle();
152163
expect(client.GET).toHaveBeenCalledTimes(1);
153164

154165
await advance(POLL_INTERVAL_MS);
@@ -161,13 +172,12 @@ describe('FaultsDashboard refresh behaviour', () => {
161172
it('does not poll while the fault stream delivers updates', async () => {
162173
const client = clientReturning([RAW_FAULT]);
163174
connect(client, true);
164-
render(
175+
await mount(
165176
<>
166177
<FaultsCountBadge />
167178
<FaultsDashboard />
168179
</>
169180
);
170-
await settle();
171181
const afterMount = client.GET.mock.calls.length;
172182

173183
await advance(POLL_INTERVAL_MS * 3);
@@ -178,11 +188,12 @@ describe('FaultsDashboard refresh behaviour', () => {
178188
it('stops polling when the last fault view unmounts', async () => {
179189
const client = clientReturning([RAW_FAULT]);
180190
connect(client, false);
181-
const { unmount } = render(<FaultsDashboard />);
182-
await settle();
191+
const { unmount } = await mount(<FaultsDashboard />);
183192
const afterMount = client.GET.mock.calls.length;
184193

185-
unmount();
194+
await act(async () => {
195+
unmount();
196+
});
186197
await advance(POLL_INTERVAL_MS * 2);
187198

188199
expect(client.GET.mock.calls.length).toBe(afterMount);

src/components/FaultsDashboard.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,8 @@ export function FaultsDashboard() {
423423
// Manual refresh handler
424424
const handleRefresh = useCallback(async () => {
425425
setIsRefreshing(true);
426-
await fetchFaults();
426+
// Forced: pressing refresh must start a read, not wait out one already running.
427+
await fetchFaults({ force: true });
427428
setIsRefreshing(false);
428429
}, [fetchFaults]);
429430

src/lib/store-faults.test.ts

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Copyright 2026 bburda
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
/**
16+
* What the shared fault list is allowed to do while it refreshes. The list has
17+
* two writers - polling and the SSE stream - and a refresh can outlive the
18+
* connection that started it, so a late answer must not decide what is on screen.
19+
*/
20+
21+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
22+
import { useAppStore } from './store';
23+
import type { Fault } from './types';
24+
25+
vi.mock('react-toastify', () => ({
26+
toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() },
27+
}));
28+
29+
function raw(overrides: Record<string, unknown> = {}) {
30+
return {
31+
fault_code: 'LIDAR_RANGE_INVALID',
32+
description: 'range invalid',
33+
severity: 2,
34+
severity_label: 'ERROR',
35+
status: 'CONFIRMED',
36+
first_occurred: 1756636800,
37+
last_occurred: 1756636800,
38+
occurrence_count: 1,
39+
reporting_sources: ['/lidar_front'],
40+
...overrides,
41+
};
42+
}
43+
44+
/** A client whose answers are under the test's control, however many are in flight. */
45+
function deferredClient() {
46+
const pending: ((items: unknown[]) => void)[] = [];
47+
const GET = vi.fn(
48+
() =>
49+
new Promise((resolve) => {
50+
pending.push((items: unknown[]) => resolve({ data: { items }, error: undefined }));
51+
})
52+
);
53+
return {
54+
client: { GET },
55+
release: (items: unknown[]) => {
56+
while (pending.length > 0) {
57+
pending.shift()!(items);
58+
}
59+
},
60+
};
61+
}
62+
63+
function clientReturning(items: unknown[]) {
64+
return { GET: vi.fn(async () => ({ data: { items }, error: undefined })) };
65+
}
66+
67+
function connected(client: unknown) {
68+
useAppStore.setState({
69+
isConnected: true,
70+
client,
71+
faults: [],
72+
faultsLoaded: false,
73+
isLoadingFaults: false,
74+
faultStreamCleanup: null,
75+
} as never);
76+
}
77+
78+
beforeEach(() => {
79+
connected(null);
80+
});
81+
82+
afterEach(() => {
83+
useAppStore.setState({ isConnected: false, client: null, faults: [] } as never);
84+
});
85+
86+
describe('fetchFaults change detection', () => {
87+
it('takes up a fault that is now reported by a different entity', async () => {
88+
connected(clientReturning([raw()]));
89+
await useAppStore.getState().fetchFaults();
90+
91+
useAppStore.setState({ client: clientReturning([raw({ reporting_sources: ['/lidar_rear'] })]) } as never);
92+
await useAppStore.getState().fetchFaults();
93+
94+
expect(useAppStore.getState().faults[0]?.entity_id).toBe('lidar_rear');
95+
});
96+
97+
it('takes up a changed fault description', async () => {
98+
connected(clientReturning([raw()]));
99+
await useAppStore.getState().fetchFaults();
100+
101+
useAppStore.setState({ client: clientReturning([raw({ description: '3 sectors blind' })]) } as never);
102+
await useAppStore.getState().fetchFaults();
103+
104+
expect(useAppStore.getState().faults[0]?.message).toBe('3 sectors blind');
105+
});
106+
});
107+
108+
describe('fetchFaults against a moving connection', () => {
109+
it('ignores an answer that arrives after the session was disconnected', async () => {
110+
const { client, release } = deferredClient();
111+
connected(client);
112+
113+
const pending = useAppStore.getState().fetchFaults();
114+
useAppStore.getState().disconnect();
115+
release([raw({ fault_code: 'FROM_PREVIOUS_GATEWAY' })]);
116+
await pending;
117+
118+
expect(useAppStore.getState().faults).toHaveLength(0);
119+
expect(useAppStore.getState().faultsLoaded).toBe(false);
120+
});
121+
122+
it('ignores an answer that the fault stream has already overtaken', async () => {
123+
const { client, release } = deferredClient();
124+
connected(client);
125+
126+
const pending = useAppStore.getState().fetchFaults();
127+
const fromStream: Fault = {
128+
code: 'STREAM_FAULT',
129+
message: 'reported over the stream',
130+
severity: 'error',
131+
status: 'active',
132+
timestamp: '2026-08-31T10:00:00.000Z',
133+
entity_id: 'lidar_front',
134+
entity_type: 'app',
135+
};
136+
useAppStore.setState({ faults: [fromStream] } as never);
137+
release([raw({ fault_code: 'FROM_THE_POLL' })]);
138+
await pending;
139+
140+
expect(useAppStore.getState().faults.map((f) => f.code)).toEqual(['STREAM_FAULT']);
141+
expect(useAppStore.getState().faultsLoaded).toBe(true);
142+
});
143+
144+
it('runs one request when several views ask for a refresh at the same time', async () => {
145+
const { client, release } = deferredClient();
146+
connected(client);
147+
148+
const first = useAppStore.getState().fetchFaults();
149+
const second = useAppStore.getState().fetchFaults();
150+
release([raw()]);
151+
await Promise.all([first, second]);
152+
153+
expect(client.GET).toHaveBeenCalledTimes(1);
154+
});
155+
156+
it('lets a forced refresh through so a cleared fault is not read back from an older answer', async () => {
157+
const { client, release } = deferredClient();
158+
connected(client);
159+
160+
const polling = useAppStore.getState().fetchFaults();
161+
const forced = useAppStore.getState().fetchFaults({ force: true });
162+
release([]);
163+
await Promise.all([polling, forced]);
164+
165+
expect(client.GET).toHaveBeenCalledTimes(2);
166+
});
167+
});

0 commit comments

Comments
 (0)