Skip to content

Commit 13aa6fc

Browse files
hayaksi1t3chguy
andauthored
Warn when an encrypted search runs before the index has finished building (#34001)
* Warn when an encrypted search runs before the index has finished building When a search runs in an encrypted room while the local Seshat index is still crawling not-yet-indexed history, results can silently come back partial. SearchWarning now subscribes to the event index's changedCheckpoint progress and shows a polite (role=status) notice while the crawl is in progress, clearing automatically the moment indexing finishes. * Scope the partial-index warning to the room being searched The warning was driven by `currentRoom() !== null`, which is a global signal: it is non-null while the crawler has any outstanding checkpoint for any room. Searching a fully-crawled room while an unrelated room was still being crawled therefore claimed the results may be incomplete when they were not. Drive it from `crawlingRooms()` instead, which reports the rooms with outstanding checkpoints by id, and pass the search scope and room id in from RoomSearchAuxPanel: a room-scoped search now asks only about that room, while an all-rooms search still reacts to any outstanding checkpoint. Using room ids throughout also avoids `currentRoom()` returning null, and so under-reporting a crawl, when the js-sdk does not know the room at the head of the queue. The `changedCheckpoint` payload only carries the globally-current room and cannot answer a per-room question, so the handler re-reads the checkpoint set. * Also warn when the searched room has not been indexed at all The crawl set cannot see a room that has no checkpoint: before the initial checkpoints have been seeded, such a room is absent from it and looks identical to one that has been fully crawled. That is the case issue #32253 describes, so ask isRoomIndexed() as well, and warn when the index holds no events for the room being searched. Only ask it while the crawler still has work outstanding. That is what the warning claims, and the index has no event for its contents changing -- changedCheckpoint fires on checkpoint transitions only, and an idle crawler is silent -- so a warning raised once the crawler had drained would never be re-evaluated and would stick. Rename the hook to useIsIndexIncomplete, as it no longer answers the narrower question of whether a crawl is in progress. Re-seed it from the checkpoint set on each scope or room change so that the previous search's answer is not left on screen while the lookup is in flight, but not on each checkpoint change, which would blink an already-earned warning off and on again. Also reword the crawlingRooms() doc, which described the set as the rooms being crawled when it holds every queued checkpoint too. --------- Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
1 parent d0a70fb commit 13aa6fc

5 files changed

Lines changed: 489 additions & 7 deletions

File tree

apps/web/src/components/views/elements/SearchWarning.tsx

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import React, { type JSX, type ReactNode } from "react";
9+
import React, { type JSX, type ReactNode, useCallback, useEffect, useState } from "react";
1010
import { logger } from "matrix-js-sdk/src/logger";
1111

1212
import EventIndexPeg from "../../../indexing/EventIndexPeg";
13+
import type EventIndex from "../../../indexing/EventIndex";
14+
import { SearchScope } from "../../../Searching";
1315
import { _t } from "../../../languageHandler";
1416
import SdkConfig from "../../../SdkConfig";
1517
import dis from "../../../dispatcher/dispatcher";
@@ -26,11 +28,131 @@ interface IProps {
2628
isRoomEncrypted?: boolean;
2729
kind: WarningKind;
2830
showLogo?: boolean;
31+
/** The scope of the search being warned about; only meaningful for {@link WarningKind.Search}. */
32+
scope?: SearchScope;
33+
/** The room being searched. Mirrors `SearchInfo.roomId`: `undefined` when searching all rooms. */
34+
roomId?: string;
2935
}
3036

31-
export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true }: IProps): JSX.Element {
37+
/**
38+
* Track whether the index is still missing history that is relevant to the given search.
39+
*
40+
* A room-scoped search is incomplete if the crawler still holds a checkpoint for the room
41+
* ({@link EventIndex.crawlingRooms}, which covers both the checkpoint being crawled right now and
42+
* those still queued behind it), or if the index holds no events for it at all
43+
* ({@link EventIndex.isRoomIndexed}) — which is how a room looks before its checkpoint has been
44+
* seeded, and is the case the checkpoint set alone cannot see.
45+
*
46+
* The second question is only asked while the crawler still has work outstanding, for two reasons.
47+
* It is what the warning claims ("your search index is still being built"), and the index has no
48+
* event for its contents changing — `changedCheckpoint` fires only on checkpoint transitions, and
49+
* an idle crawler is silent — so a warning raised once the crawler has drained would never be
50+
* re-evaluated and would stick.
51+
*
52+
* Neither signal proves completeness: `isRoomIndexed` reports only that the index holds *some*
53+
* events for a room, not all of them, and a room has no checkpoint if it never had a
54+
* back-pagination token to crawl from or if its checkpoint was dropped because the server rejected
55+
* the request. So this under-warns rather than over-warns.
56+
*
57+
* An all-rooms search cannot ask the per-room question, so it uses the checkpoint set alone.
58+
*
59+
* The `changedCheckpoint` payload carries only the globally-current room and so cannot answer a
60+
* per-room question: we re-read the index on each event rather than trust it.
61+
*
62+
* @param index The event index to observe, or `null` if there is no index.
63+
* @param scope The scope of the search, if this warning is being rendered for one.
64+
* @param roomId The room being searched, or `undefined` when searching all rooms.
65+
* @returns `true` while the index is known to be missing history for the search, `false` otherwise.
66+
*/
67+
function useIsIndexIncomplete(index: EventIndex | null, scope?: SearchScope, roomId?: string): boolean {
68+
const readCheckpoints = useCallback((): { relevant: boolean; anyOutstanding: boolean } => {
69+
if (!index) return { relevant: false, anyOutstanding: false };
70+
const { crawlingRooms } = index.crawlingRooms();
71+
// Fall back to the global check when we don't know which room is being searched: the room
72+
// id may still be undefined while a room alias is being resolved.
73+
const roomScoped = scope === SearchScope.Room && roomId !== undefined;
74+
return {
75+
relevant: roomScoped ? crawlingRooms.has(roomId) : crawlingRooms.size > 0,
76+
anyOutstanding: crawlingRooms.size > 0,
77+
};
78+
}, [index, scope, roomId]);
79+
80+
// The checkpoint half of the answer is known synchronously, so seed from it rather than
81+
// rendering an unwarned search for a room we already know is being crawled.
82+
const [incomplete, setIncomplete] = useState<boolean>(() => readCheckpoints().relevant);
83+
84+
useEffect(() => {
85+
if (!index) {
86+
setIncomplete(false);
87+
return;
88+
}
89+
90+
// Guards against a slow isRoomIndexed() response overwriting a newer one, or landing after
91+
// the scope changed or the component unmounted.
92+
let generation = 0;
93+
94+
// Answer this scope and room from the checkpoint set up front, so that the previous
95+
// search's result is not left on screen while the first lookup below is in flight. Doing
96+
// this per effect run rather than per event matters: a checkpoint change is not a new
97+
// question, and resetting on one would blink an already-earned warning off and on again.
98+
setIncomplete(readCheckpoints().relevant);
99+
100+
const update = async (): Promise<void> => {
101+
const current = ++generation;
102+
const { relevant, anyOutstanding } = readCheckpoints();
103+
104+
if (relevant) {
105+
setIncomplete(true);
106+
return;
107+
}
108+
if (!anyOutstanding || scope !== SearchScope.Room || roomId === undefined) {
109+
setIncomplete(false);
110+
return;
111+
}
112+
113+
// Nothing is queued for this room yet, but the index may hold nothing for it at all.
114+
// `undefined` means there is no index manager to ask, which is not evidence either way.
115+
const indexed = await index.isRoomIndexed(roomId);
116+
if (current === generation) setIncomplete(indexed === false);
117+
};
118+
119+
const onChangedCheckpoint = (): void => {
120+
void update();
121+
};
122+
123+
// Re-sync in case the crawl state changed between the initial render and the subscription.
124+
onChangedCheckpoint();
125+
index.on("changedCheckpoint", onChangedCheckpoint);
126+
127+
return () => {
128+
generation++;
129+
index.removeListener("changedCheckpoint", onChangedCheckpoint);
130+
};
131+
}, [index, scope, roomId, readCheckpoints]);
132+
133+
return incomplete;
134+
}
135+
136+
export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true, scope, roomId }: IProps): JSX.Element {
137+
const eventIndex = EventIndexPeg.get();
138+
const indexIncomplete = useIsIndexIncomplete(eventIndex, scope, roomId);
139+
32140
if (!isRoomEncrypted) return <></>;
33-
if (EventIndexPeg.get()) return <></>;
141+
142+
if (eventIndex) {
143+
// The index is still missing history for this search, so it may silently return partial
144+
// results (#32253). Warn the user.
145+
if (indexIncomplete && kind === WarningKind.Search) {
146+
// This warning appears dynamically while a search panel is already open (the crawler
147+
// finishes draining mid-session), so mark it as a polite live region for screen readers.
148+
return (
149+
<div className="mx_SearchWarning" role="status">
150+
<span>{_t("seshat|warning_kind_search_partial")}</span>
151+
</div>
152+
);
153+
}
154+
return <></>;
155+
}
34156

35157
if (EventIndexPeg.error) {
36158
return (

apps/web/src/components/views/rooms/RoomSearchAuxPanel.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,13 @@ const RoomSearchAuxPanel: React.FC<Props> = ({ searchInfo, isRoomEncrypted, onSe
4545
) : (
4646
<InlineSpinner />
4747
)}
48-
<SearchWarning kind={WarningKind.Search} isRoomEncrypted={isRoomEncrypted} showLogo={false} />
48+
<SearchWarning
49+
kind={WarningKind.Search}
50+
isRoomEncrypted={isRoomEncrypted}
51+
showLogo={false}
52+
scope={scope}
53+
roomId={searchInfo?.roomId}
54+
/>
4955
</div>
5056
</div>
5157
<div className="mx_RoomSearchAuxPanel_buttons">

apps/web/src/i18n/strings/en_EN.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2434,7 +2434,8 @@
24342434
"warning_kind_files": "This version of %(brand)s does not support viewing some encrypted files",
24352435
"warning_kind_files_app": "Use the <a>Desktop app</a> to see all encrypted files",
24362436
"warning_kind_search": "This version of %(brand)s does not support searching encrypted messages",
2437-
"warning_kind_search_app": "Use the <a>Desktop app</a> to search encrypted messages"
2437+
"warning_kind_search_app": "Use the <a>Desktop app</a> to search encrypted messages",
2438+
"warning_kind_search_partial": "Results may be incomplete because your search index is still being built."
24382439
},
24392440
"setting": {
24402441
"help_about": {

apps/web/src/indexing/EventIndex.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,10 @@ export default class EventIndex extends EventEmitter {
992992
}
993993

994994
public crawlingRooms(): {
995-
/** The rooms that we are currently crawling. */
995+
/**
996+
* The rooms with an outstanding crawler checkpoint: the one being crawled right now, and
997+
* those still queued behind it.
998+
*/
996999
crawlingRooms: Set<string>;
9971000

9981001
/** All the encrypted rooms known by the MatrixClient. */

0 commit comments

Comments
 (0)