Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 49 additions & 8 deletions src/features/instance/status/analytics/StatusTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs';
import type { InstanceClientIdConfig, InstanceTypeConfig } from '@/config/instanceClientConfig.ts';
import { useSystemTheme } from '@/hooks/useSystemTheme';
import { ANALYTICS_QUERY_KEY_PREFIX } from '@/integrations/api/instance/status/getAnalytics.ts';
import { useQueryClient } from '@tanstack/react-query';
import { useNavigate, useSearch } from '@tanstack/react-router';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { AnalyticsOnboardingHint } from './components/AnalyticsOnboardingHint.tsx';
import { TimeRangePicker } from './components/TimeRangePicker.tsx';
import { type AnalyticsContextValue, AnalyticsProvider } from './context/AnalyticsContext.tsx';
Expand Down Expand Up @@ -76,9 +78,49 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) {
? Number(raw.refresh)
: DEFAULT_REFRESH_MS;

// Manual refresh ticks bump this to force a fresh window when the user
// clicks the refresh button.
// The analytics window is a fixed [start, end] snapshot baked into every
// panel's query key, so "refresh" means sliding the window forward to a
// new snapshot — re-fetching the old one could only return the same data.
// `tick` bumps recompute the window below; the interval drives it when
// auto-refresh is on, and the manual refresh button shares the same path.
const [tick, setTick] = useState(0);
const queryClient = useQueryClient();
const lastRefreshRef = useRef(Date.now());
const refresh = useCallback(() => {
lastRefreshRef.current = Date.now();
setTick((t) => t + 1);
}, []);

useEffect(() => {
if (refreshMs <= 0) { return; }
// `tick` in the deps restarts the interval on EVERY refresh (scheduled,
// manual, or visibility catch-up), so the next scheduled tick is always
// exactly refreshMs after the last actual refresh — no elapsed-time
// bookkeeping and no double-fetch when refresh sources land close
// together.
void tick;
// Tick guards:
// - hidden tab: fetching for an invisible dashboard is pure load on
// the customer's Harper; the visibility handler below catches up
// once on return if at least one full period elapsed.
// - in-flight: when Harper is slower than the cadence, sliding the
// window again would key a fresh POST batch on top of the running
// one (new key = no dedupe); skip until the current batch settles.
const canTick = () =>
!document.hidden
&& queryClient.isFetching({ queryKey: [ANALYTICS_QUERY_KEY_PREFIX] }) === 0;
const intervalId = setInterval(() => {
if (canTick()) { refresh(); }
}, refreshMs);
const onVisibilityChange = () => {
if (canTick() && Date.now() - lastRefreshRef.current >= refreshMs) { refresh(); }
};
document.addEventListener('visibilitychange', onVisibilityChange);
return () => {
clearInterval(intervalId);
document.removeEventListener('visibilitychange', onVisibilityChange);
};
}, [refreshMs, refresh, queryClient, tick]);

const theme = useSystemTheme();

Expand Down Expand Up @@ -107,17 +149,16 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) {
const preset = getPreset(presetId);
const endTime = Date.now();
const startTime = endTime - preset.durationMs;
// `tick` participates in memo deps so a manual refresh produces a fresh
// window even if the user did not change presets.
// `tick` participates in memo deps so every refresh (interval or manual)
// produces a fresh window even if the user did not change presets.
void tick;
return {
timeRange: { startTime, endTime },
bucketMs: preset.bucketMs,
refreshIntervalMs: refreshMs,
theme,
instanceParams,
};
}, [presetId, refreshMs, theme, instanceParams, tick]);
}, [presetId, theme, instanceParams, tick]);

const showTimePicker = tab !== 'overview';
const picker = showTimePicker
Expand All @@ -127,7 +168,7 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) {
onPresetChange={updatePreset}
refreshMs={refreshMs}
onRefreshChange={updateRefreshMs}
onManualRefresh={() => setTick((t) => t + 1)}
onManualRefresh={refresh}
/>
)
: null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
// @vitest-environment happy-dom
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { act, cleanup, render } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

// Regression coverage for the frozen-refresh-window bug (#1439): the analytics
// window is baked into every panel's query key, so auto-refresh must slide the
// window forward — re-polling a fixed [start, end] can only return stale data.
// These tests observe the windows each panel requests via the mocked records
// hook and assert the clock in StatusTabsInner advances them.

const seenWindows: { startTime: number; endTime: number }[] = [];
vi.mock('../hooks/useAnalyticsRecords.ts', () => ({
useAnalyticsRecords: (args: { startTime: number; endTime: number }) => {
seenWindows.push({ startTime: args.startTime, endTime: args.endTime });
return {
data: [],
isLoading: false,
isError: false,
error: null,
isEmpty: true,
fieldKeys: new Set<string>(),
missingFields: [],
refetch: vi.fn(),
};
},
}));

vi.mock('../hooks/useAnalyticsCapability.ts', () => ({
useAnalyticsCapability: () => ({ supported: true, isLoading: false, retry: vi.fn() }),
}));

let currentSearch: Record<string, unknown> = {};
const navigateMock = vi.fn(async (opts: { search?: unknown }) => {
if (typeof opts.search === 'object' && opts.search !== null) {
currentSearch = { ...(opts.search as Record<string, unknown>) };
} else {
currentSearch = {};
}
});
vi.mock('@tanstack/react-router', () => ({
useSearch: () => currentSearch,
useNavigate: () => navigateMock,
}));

import { DEFAULT_REFRESH_MS } from '../context/timePresets.ts';
import { StatusTabs } from '../StatusTabs.tsx';

const instanceParams = {
instanceClient: { post: vi.fn(async () => ({ data: [] })) } as never,
entityId: 'inst-A' as never,
entityType: 'instance' as const,
};

function mount() {
const client = new QueryClient({
defaultOptions: { queries: { retry: false, gcTime: 0 } },
});
return render(
<QueryClientProvider client={client}>
<StatusTabs instanceParams={instanceParams} isLocalStudio={false} />
</QueryClientProvider>,
);
}

function latestEndTime(): number {
return Math.max(...seenWindows.map((w) => w.endTime));
}

/** Force `document.hidden` (happy-dom defaults it to false, non-configurable
* access goes through visibilityState — define both). */
function setDocumentHidden(hidden: boolean) {
Object.defineProperty(document, 'hidden', { configurable: true, get: () => hidden });
Object.defineProperty(document, 'visibilityState', {
configurable: true,
get: () => (hidden ? 'hidden' : 'visible'),
});
}

beforeEach(() => {
vi.useFakeTimers();
currentSearch = {};
seenWindows.length = 0;
navigateMock.mockClear();
setDocumentHidden(false);
Object.defineProperty(window, 'matchMedia', {
writable: true,
configurable: true,
value: vi.fn().mockImplementation((query: string) => ({
matches: false,
media: query,
onchange: null,
addEventListener: vi.fn(),
removeEventListener: vi.fn(),
addListener: vi.fn(),
removeListener: vi.fn(),
dispatchEvent: vi.fn(),
})),
});
});

afterEach(() => {
cleanup();
vi.useRealTimers();
});

describe('StatusTabs sliding refresh window', () => {
it('advances the window by one refresh period per tick', async () => {
mount();
const initialEnd = latestEndTime();
expect(initialEnd).toBeGreaterThan(0);

await act(async () => {
await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS + 5);
});
const afterOneTick = latestEndTime();
expect(afterOneTick).toBeGreaterThanOrEqual(initialEnd + DEFAULT_REFRESH_MS);

await act(async () => {
await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS);
});
expect(latestEndTime()).toBeGreaterThanOrEqual(afterOneTick + DEFAULT_REFRESH_MS);
});

it('does not tick while auto-refresh is off (refresh=0)', async () => {
currentSearch = { refresh: 0 };
mount();
const initialEnd = latestEndTime();

await act(async () => {
await vi.advanceTimersByTimeAsync(10 * DEFAULT_REFRESH_MS);
});
expect(latestEndTime()).toBe(initialEnd);
});

it('pauses while the browser tab is hidden and catches up once on return', async () => {
mount();
const initialEnd = latestEndTime();

setDocumentHidden(true);
await act(async () => {
await vi.advanceTimersByTimeAsync(2.5 * DEFAULT_REFRESH_MS);
});
// Hidden: interval fires but skips the refresh — window stays frozen.
expect(latestEndTime()).toBe(initialEnd);

setDocumentHidden(false);
await act(async () => {
document.dispatchEvent(new Event('visibilitychange'));
});
// Visible again: one catch-up tick (not one per missed period).
const afterReturn = latestEndTime();
expect(afterReturn).toBeGreaterThanOrEqual(initialEnd + 2 * DEFAULT_REFRESH_MS);

// The catch-up restarts the interval, so no tick fires at the old
// half-period-away boundary (no back-to-back burst)…
await act(async () => {
await vi.advanceTimersByTimeAsync(0.5 * DEFAULT_REFRESH_MS + 5);
});
expect(latestEndTime()).toBe(afterReturn);

// …and the next tick fires a full period after the catch-up.
await act(async () => {
await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS);
});
expect(latestEndTime()).toBeGreaterThanOrEqual(afterReturn + DEFAULT_REFRESH_MS);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export type AnalyticsTheme = 'light' | 'dark';
export interface AnalyticsContextValue {
timeRange: TimeRange;
bucketMs: number;
refreshIntervalMs: number;
theme: AnalyticsTheme;
instanceParams: InstanceClientIdConfig & InstanceTypeConfig;
}
Expand All @@ -24,7 +23,6 @@ export function AnalyticsProvider({ value, children }: ProviderProps) {
value.timeRange.startTime,
value.timeRange.endTime,
value.bucketMs,
value.refreshIntervalMs,
value.theme,
value.instanceParams.entityId,
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: makeInstanceParams(rows),
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -66,7 +65,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: makeInstanceParams([]),
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -88,7 +86,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: makeInstanceParams([]),
refetchIntervalMs: 0,
requiredFields: ['p95', 'path'],
}),
{ wrapper: Wrapper },
Expand All @@ -111,7 +108,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: makeInstanceParams(rows),
refetchIntervalMs: 0,
requiredFields: ['p95', 'p99'],
}),
{ wrapper: Wrapper },
Expand All @@ -133,7 +129,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: paramsA,
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -144,7 +139,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: paramsB,
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand Down Expand Up @@ -172,7 +166,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: makeInstanceParams([], { throwError: new Error('boom') }),
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -191,7 +184,6 @@ describe('useAnalyticsRecords', () => {
endTime: 10_000,
instanceParams: params,
conditions: [{ attribute: 'path', value: '/api/x' }],
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -215,7 +207,6 @@ describe('useAnalyticsRecords', () => {
endTime: 10_000,
instanceParams: params,
bucketMs: 60_000,
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -239,7 +230,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: makeInstanceParams([]),
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand All @@ -260,7 +250,6 @@ describe('useAnalyticsRecords', () => {
startTime: 100,
endTime: 200,
instanceParams: makeInstanceParams(driftRows),
refetchIntervalMs: 0,
requiredFields: ['user'],
}),
{ wrapper: W2 },
Expand All @@ -277,7 +266,6 @@ describe('useAnalyticsRecords', () => {
startTime: 200,
endTime: 300,
instanceParams: makeInstanceParams([]),
refetchIntervalMs: 0,
requiredFields: ['user'],
}),
{ wrapper: W3 },
Expand All @@ -294,7 +282,7 @@ describe('useAnalyticsRecords', () => {
warn.mockRestore();
});

it('refetchIntervalMs=0 disables polling (no extra fetch after initial)', async () => {
it('never polls — no extra fetch after the initial one', async () => {
vi.useFakeTimers();
try {
const { Wrapper } = wrapper();
Expand All @@ -306,7 +294,6 @@ describe('useAnalyticsRecords', () => {
startTime: 0,
endTime: 10_000,
instanceParams: params,
refetchIntervalMs: 0,
}),
{ wrapper: Wrapper },
);
Expand Down Expand Up @@ -334,7 +321,6 @@ describe('useAnalyticsRecords', () => {
startTime: args.startTime,
endTime: args.endTime,
instanceParams: params,
refetchIntervalMs: 0,
}),
{
wrapper: Wrapper,
Expand Down
Loading