fix: the diagnostics tab keeps its focus, its level and its consent

Deep-review round 1 of E22.

- After a 429 the stream sent one POST per new event while a full batch
  waited; it now stays quiet for 15 s. Past the queue cap the oldest
  events go, as the comment always said, a refused batch included.
- A batch refused after the level was lowered came back into the queue
  of a later consent in the same tab; each consent is its own session.
- A `me` fetch that left before a level or preference change could put the
  old value back for up to a minute. `updateMeCache` writes the cache and
  cancels that fetch; usePref goes through it too.
- The consent box's buttons and "revert now" took the focus with them to
  <body>; it returns to the level picker. Watched until the button is
  gone — in Chrome the saved level arrives in two renders, and a one-shot
  check spent itself on the first (jsdom never showed it).
- The countdown line is no longer a live region re-read every minute.
- "Saját napló" rows were keyed by index on a newest-first list, so every
  new event remounted every row and dropped the focus off a row button.
This commit is contained in:
2026-09-17 03:53:40 +02:00
parent 4bfba4aa86
commit f92cd9017f
8 changed files with 220 additions and 30 deletions
@@ -63,6 +63,7 @@ function mount(me: Partial<Me>) {
<DiagnosticsSettings />
</QueryClientProvider>,
);
return qc;
}
const levelButton = (name: string) =>
@@ -113,6 +114,58 @@ describe("DiagnosticsSettings", () => {
await waitFor(() => expect(setLevel).toHaveBeenCalledWith("INFO"));
});
it("the focus goes back to the level picker when the box it was in goes away", async () => {
mount({});
fireEvent.click(levelButton("settings.diagnostics.levels.DEBUG"));
const cancel = screen.getByRole("button", { name: "settings.diagnostics.consent.cancel" });
cancel.focus();
fireEvent.click(cancel);
expect(document.activeElement).toBe(levelButton("settings.diagnostics.levels.INFO"));
fireEvent.click(levelButton("settings.diagnostics.levels.TRACE"));
const confirm = screen.getByTestId("diagnostics-consent-confirm");
confirm.focus();
fireEvent.click(confirm);
await waitFor(() => expect(screen.getByTestId("diagnostics-status")).toBeTruthy());
expect(document.activeElement).toBe(levelButton("settings.diagnostics.levels.TRACE"));
const revert = screen.getByRole("button", { name: "settings.diagnostics.revert" });
revert.focus();
fireEvent.click(revert);
await waitFor(() => expect(screen.queryByTestId("diagnostics-status")).toBeNull());
expect(document.activeElement).toBe(levelButton("settings.diagnostics.levels.INFO"));
});
it("the ticking status line is not a live region", async () => {
const until = new Date(Date.now() + 3_600_000).toISOString();
mount({ diagnostics: { level: "DEBUG", until, retention_days: 7 } });
const status = screen.getByTestId("diagnostics-status");
expect(status.closest('[role="status"], [aria-live]')).toBeNull();
});
it("a `me` fetch that left before the change cannot put the old level back", async () => {
const qc = mount({});
let answerStale: (me: unknown) => void = () => {};
const stale = qc
.fetchQuery({
queryKey: qk.me(),
queryFn: () => new Promise((resolve) => (answerStale = resolve)),
})
.catch(() => null);
fireEvent.click(levelButton("settings.diagnostics.levels.TRACE"));
fireEvent.click(screen.getByTestId("diagnostics-consent-confirm"));
await waitFor(() => expect(screen.getByTestId("diagnostics-status")).toBeTruthy());
answerStale({
id: 1,
is_demo: false,
preferences: {},
diagnostics: { level: "INFO", until: null, retention_days: 7 },
});
await stale;
expect(qc.getQueryData<Me>(qk.me())?.diagnostics.level).toBe("TRACE");
expect(screen.getByTestId("diagnostics-status")).toBeTruthy();
});
it("a lapsed level shows as the base level", () => {
const until = new Date(Date.now() - 60_000).toISOString();
mount({ diagnostics: { level: "TRACE", until, retention_days: 7 } });
+43 -16
View File
@@ -1,11 +1,10 @@
import { useEffect, useState } from "react";
import { useEffect, useRef, useState } from "react";
import { useMutation, useQueryClient } from "@tanstack/react-query";
import { useTranslation } from "react-i18next";
import { api, type DiagnosticsLevel, type Me } from "../lib/api";
import { api, type DiagnosticsLevel } from "../lib/api";
import { levelInForce } from "../lib/diagnostics";
import { qk } from "../lib/queryKeys";
import { useMe } from "../lib/useMe";
import { updateMeCache, useMe } from "../lib/useMe";
import { Button } from "./ui/Button";
import InlineNotice from "./ui/InlineNotice";
import SegmentedToggle from "./ui/SegmentedToggle";
@@ -42,17 +41,42 @@ export default function DiagnosticsSettings() {
const [pending, setPending] = useState<DiagnosticsLevel | null>(null);
const remaining = useRemaining(current === "INFO" ? null : (state?.until ?? null));
// The consent box and the status line each go away with the button that closed them — the one
// holding the focus. Put it back on the level picker (its chosen option) rather than let it fall
// to <body>, where the next Tab starts over from the top of the page. Only when it did fall: a
// user who moved on while the request was out keeps their place.
//
// Watched until that button is actually GONE, not for one render: a saved level arrives as two
// updates (the `me` cache, then the closed box), and the first render still has the button in the
// page — measured in Chrome, where a one-shot check spent itself there and the focus then fell.
const pickerRef = useRef<HTMLDivElement>(null);
const closedFrom = useRef<Element | null>(null);
useEffect(() => {
const from = closedFrom.current;
if (!from || from.isConnected) return;
closedFrom.current = null;
if (document.activeElement && document.activeElement !== document.body) return;
pickerRef.current?.querySelector<HTMLElement>('[aria-pressed="true"]')?.focus();
});
const watchFocus = () => {
closedFrom.current = document.activeElement;
};
const change = useMutation({
mutationFn: api.setDiagnosticsLevel,
meta: { failTitle: "settings.diagnostics.failed" },
onSuccess: (result) => {
qc.setQueryData<Me | null>(qk.me(), (prev) =>
prev ? { ...prev, diagnostics: result.diagnostics } : prev,
);
watchFocus();
updateMeCache(qc, (prev) => ({ ...prev, diagnostics: result.diagnostics }));
setPending(null);
},
});
const cancel = () => {
watchFocus();
setPending(null);
};
const choose = (level: DiagnosticsLevel) => {
if (level === "INFO") {
setPending(null);
@@ -74,13 +98,15 @@ export default function DiagnosticsSettings() {
label={t("settings.diagnostics.level")}
hint={t("settings.diagnostics.levelHint")}
>
<SegmentedToggle
size="md"
ariaLabel={t("settings.diagnostics.level")}
value={pending ?? current}
onChange={choose}
options={LEVELS.map((level) => ({ value: level, label: levelName(level) }))}
/>
<div ref={pickerRef}>
<SegmentedToggle
size="md"
ariaLabel={t("settings.diagnostics.level")}
value={pending ?? current}
onChange={choose}
options={LEVELS.map((level) => ({ value: level, label: levelName(level) }))}
/>
</div>
</SettingRow>
)}
@@ -111,7 +137,7 @@ export default function DiagnosticsSettings() {
</li>
</ul>
<div className="flex flex-wrap justify-end gap-2">
<Button variant="ghost" size="sm" onClick={() => setPending(null)}>
<Button variant="ghost" size="sm" onClick={cancel}>
{t("settings.diagnostics.consent.cancel")}
</Button>
<Button
@@ -127,9 +153,10 @@ export default function DiagnosticsSettings() {
)}
{current !== "INFO" && !pending && remaining && (
// Not a live region: the countdown changes every minute, and a screen reader would read
// the whole line out again each time for as long as the page is open.
<div
data-testid="diagnostics-status"
role="status"
className="mt-2 flex flex-wrap items-center justify-between gap-2 rounded-xl border border-warn/40 bg-warn/10 px-3 py-2 text-sm"
>
<span className="inline-flex items-center gap-2">
@@ -56,6 +56,20 @@ describe("MyLogViewer", () => {
expect(rows()[0]).toContain("app.uncaught");
});
it("a row keeps its place and the focus as new events arrive above it", async () => {
render(<MyLogViewer />);
const row = within(screen.getByTestId("my-log-lines")).getAllByRole("button")[0]!;
row.focus();
fireEvent.click(row); // at Részletes this click is itself a new event in real use
await act(async () => {
log.debug("ui.click", "a newer event");
await new Promise((r) => requestAnimationFrame(() => r(null)));
});
expect(row.isConnected).toBe(true);
expect(document.activeElement).toBe(row);
expect(row.getAttribute("aria-expanded")).toBe("true");
});
it("filters by level group and by text, and copies only what is shown", async () => {
render(<MyLogViewer />);
fireEvent.click(screen.getByRole("button", { name: /settings\.myLog\.filters\.warning/ }));
+17 -2
View File
@@ -51,6 +51,21 @@ function useLogEvents(): LogEvent[] {
return list;
}
// A row's React key: the event OBJECT's own number, handed out the first time it is drawn. The ring
// keeps the same objects, so the key stays put as the list grows at the top — an index key shifted
// every row on every new event, remounting all of them and dropping the focus off the row button
// (at Részletes a click on a row logs a line itself, so it happened on every click).
const rowIds = new WeakMap<LogEvent, number>();
let nextRowId = 0;
const rowKey = (event: LogEvent) => {
let id = rowIds.get(event);
if (id === undefined) {
id = nextRowId++;
rowIds.set(event, id);
}
return id;
};
const matches = (event: LogEvent, needle: string) =>
!needle ||
[event.event, event.msg, event.request_id ?? "", event.attrs ? JSON.stringify(event.attrs) : ""]
@@ -151,11 +166,11 @@ export default function MyLogViewer() {
{all.length === 0 ? t("settings.myLog.emptyAll") : t("settings.myLog.empty")}
</p>
) : (
shown.slice(0, ROWS_SHOWN).map((e, i) => {
shown.slice(0, ROWS_SHOWN).map((e) => {
const tag = LEVEL_TAG[e.level];
const isOpen = open === e;
return (
<div key={`${e.ts}-${i}`} className="border-b border-border/50 last:border-b-0">
<div key={rowKey(e)} className="border-b border-border/50 last:border-b-0">
<button
type="button"
aria-expanded={isOpen}
+56 -2
View File
@@ -130,12 +130,66 @@ describe("diagnostics level", () => {
d.log.debug("ui.click", "kept for later");
await vi.advanceTimersByTimeAsync(5_000);
answer = () => Promise.reject(new d.HttpError(409, "off"));
await vi.advanceTimersByTimeAsync(5_000);
expect(codes(sent[1])).toEqual(codes(sent[0])); // the same batch, retried after the 429
await vi.advanceTimersByTimeAsync(15_000);
expect(codes(sent[1])).toEqual(codes(sent[0])); // the same batch, retried after the back-off
expect(lapsed).toHaveBeenCalledOnce();
expect(d.enabled("DEBUG")).toBe(false);
});
it("after a 429 nothing is sent until the back-off is over, however full the queue", async () => {
const d = await fresh();
d.applyDiagnostics(raised("DEBUG"), () => {});
answer = () => Promise.reject(new d.HttpError(429, "slow down"));
for (let i = 0; i < 99; i++) d.log.debug("ui.click", `click ${i}`);
await vi.advanceTimersByTimeAsync(0);
expect(sent).toHaveLength(1); // the full batch went at once, and was refused
answer = () => Promise.resolve({ ok: true, stored: 0 });
for (let i = 0; i < 50; i++) d.log.debug("ui.click", `more ${i}`);
await vi.advanceTimersByTimeAsync(14_000);
expect(sent).toHaveLength(1); // one POST per new event is exactly the storm to avoid
await vi.advanceTimersByTimeAsync(1_000);
expect(codes(sent[1])).toEqual(codes(sent[0]));
});
it("past the queue cap the oldest events go, a refused batch included", async () => {
const d = await fresh();
d.applyDiagnostics(raised("DEBUG"), () => {});
answer = () => Promise.reject(new d.HttpError(429, "slow down"));
for (let i = 0; i < 99; i++) d.log.debug("ui.click", `click ${i}`);
await vi.advanceTimersByTimeAsync(0);
answer = () => Promise.resolve({ ok: true, stored: 0 });
for (let i = 99; i < 1_100; i++) d.log.debug("ui.click", `click ${i}`);
await vi.advanceTimersByTimeAsync(60_000);
const msgs = sent
.slice(1)
.flat()
.map((e) => (e as { msg: string }).msg);
expect(msgs).toHaveLength(1_000);
expect(msgs[0]).toBe("click 100");
expect(msgs[msgs.length - 1]).toBe("click 1099");
});
it("a batch refused after the level was lowered is not sent under a later consent", async () => {
const d = await fresh();
let refuse: (err: unknown) => void = () => {};
answer = () => new Promise((_, reject) => (refuse = reject));
d.applyDiagnostics(raised("DEBUG"), () => {});
d.log.debug("ui.click", "under the first consent");
await vi.advanceTimersByTimeAsync(5_000); // in flight
d.applyDiagnostics({ level: "INFO", until: null, retention_days: 7 }, () => {});
refuse(new d.HttpError(429, "slow down"));
await vi.advanceTimersByTimeAsync(0);
answer = () => Promise.resolve({ ok: true, stored: 0 });
d.applyDiagnostics(raised("TRACE"), () => {});
await vi.advanceTimersByTimeAsync(60_000);
const later = sent
.slice(1)
.flat()
.map((e) => (e as { msg: string }).msg);
expect(later).not.toContain("under the first consent");
expect(later).toContain("diagnostics level raised");
});
it("a closing tab sends what is still waiting with a keepalive request", async () => {
const d = await fresh();
d.applyDiagnostics(raised("DEBUG"), () => {});
+24 -6
View File
@@ -10,7 +10,8 @@
// refetch), or when the server answers that it is off.
// A lost batch is dropped, not retried in a loop: this is a diagnostic stream, not a ledger, and a
// retry storm against a struggling server is the last thing a diagnosis needs. The one exception is
// being told to slow down (429): that batch goes back to the front of the queue.
// being told to slow down (429): that batch goes back to the front of the queue, and nothing is sent
// for BACKOFF_MS — not even when a full batch is waiting, or every new event would send one.
import { api, DIAGNOSTICS_EVENTS_PATH, HttpError, type DiagnosticsState } from "./api";
import { log, setLevel, subscribe, type LogEvent } from "./log";
@@ -19,6 +20,8 @@ const FLUSH_MS = 5_000;
const BATCH_MAX = 100;
/** Waiting events kept while the server is slow or away; past it the oldest go. */
const QUEUE_MAX = 1_000;
/** Quiet time after a 429. The server counts batches per minute, per account — every tab of it. */
const BACKOFF_MS = 15_000;
/** The level actually in force: the consented one until it lapses. */
export function levelInForce(state: DiagnosticsState | null | undefined, now = Date.now()) {
@@ -74,16 +77,27 @@ let queue: LogEvent[] = [];
let unsubscribe: (() => void) | null = null;
let flushTimer: number | null = null;
let inFlight = false;
let quietUntil = 0;
// Which consent a batch was taken under. A batch still in flight when the level is lowered must not
// come back into the queue of a LATER consent: it was captured under the one just withdrawn.
let session = 0;
const isOwnRequest = (event: LogEvent) =>
event.event === "http.request" && event.attrs?.path === DIAGNOSTICS_EVENTS_PATH;
const keepNewest = () => {
if (queue.length > QUEUE_MAX) queue.splice(0, queue.length - QUEUE_MAX);
};
function startStreaming(): void {
if (unsubscribe) return;
session += 1;
queue = [];
quietUntil = 0;
unsubscribe = subscribe((event) => {
if (!event || isOwnRequest(event)) return;
queue.push(event);
if (queue.length > QUEUE_MAX) queue.splice(0, queue.length - QUEUE_MAX);
keepNewest();
if (queue.length >= BATCH_MAX) void flush();
});
flushTimer = window.setInterval(() => void flush(), FLUSH_MS);
@@ -101,16 +115,20 @@ function stopStreaming(): void {
}
async function flush(): Promise<void> {
if (inFlight || !queue.length) return;
if (inFlight || !queue.length || Date.now() < quietUntil) return;
inFlight = true;
const takenIn = session;
const batch = queue.splice(0, BATCH_MAX);
try {
await api.sendLogEvents(batch);
} catch (err) {
if (err instanceof HttpError && err.status === 429) {
queue.unshift(...batch);
if (queue.length > QUEUE_MAX) queue.length = QUEUE_MAX;
} else if (err instanceof HttpError && err.status === 409) {
quietUntil = Date.now() + BACKOFF_MS;
if (takenIn === session && unsubscribe) {
queue.unshift(...batch);
keepNewest();
}
} else if (err instanceof HttpError && err.status === 409 && takenIn === session) {
lapse(); // the server has it off: withdrawn in another tab, or lapsed by its clock
}
} finally {
+11 -1
View File
@@ -1,7 +1,17 @@
import { useQuery } from "@tanstack/react-query";
import { useQuery, type QueryClient } from "@tanstack/react-query";
import { qk } from "./queryKeys";
import { api, type Me } from "./api";
/** Write a change the SERVER already has (or is about to) into the `me` cache — and cancel a `me`
* fetch still in flight. That fetch may have left before the change was saved (BootGate's 60 s poll,
* a refetch on window focus); answering after the write, it would put the old value back and hold it
* until the next poll. Cancelling reverts the query to its latest state, which is this write:
* react-query takes a manual write as the state to revert to. */
export function updateMeCache(qc: QueryClient, update: (me: Me) => Me): void {
qc.setQueryData<Me | null>(qk.me(), (me) => (me ? update(me) : me));
void qc.cancelQueries({ queryKey: qk.me() });
}
// The signed-in account, read from the qk.me() query cache. BootGate owns the FETCHING query (gated on
// setup state) and App renders modules only once it has resolved to a real user — so anywhere inside the
// authenticated module tree the cache is always populated. `enabled:false` makes this a passive
+2 -3
View File
@@ -5,6 +5,7 @@ import { api } from "./api";
import type { Me } from "./api/account";
import { readAccount, writeAccount } from "./storage";
import { notifyActionError } from "./actionErrors";
import { updateMeCache } from "./useMe";
import i18n from "../i18n";
// The local state + cache already show the new value, so a refused save is otherwise invisible until
@@ -45,9 +46,7 @@ function readServerPref(me: Me | null | undefined, key: string): unknown {
// (their sole difference is WHEN the server write fires — immediately vs debounced), so the cache-merge
// shape lives in ONE place.
function mergePrefsCache(qc: QueryClient, patch: Record<string, unknown>): void {
qc.setQueryData<Me | null>(qk.me(), (m) =>
m ? { ...m, preferences: { ...m.preferences, ...patch } } : m,
);
updateMeCache(qc, (m) => ({ ...m, preferences: { ...m.preferences, ...patch } }));
}
/** The raw preference persister: save an arbitrary partial preferences patch to the server and merge