print better errors in the search view instead of a blocking modal (#29724)

* print better errors in the search view instead of a blocking modal

* update tests and i18n

* fix unused variable

* fix unused variable again
This commit is contained in:
Julien CLEMENT 2025-04-14 15:36:34 +02:00 committed by GitHub
parent 7ce0a76414
commit 475e449e81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 24 additions and 22 deletions

View File

@ -718,4 +718,8 @@ export interface SearchInfo {
* The total count of matching results as returned by the backend. * The total count of matching results as returned by the backend.
*/ */
count?: number; count?: number;
/**
* Describe the error if any occured.
*/
error?: Error;
} }

View File

@ -21,8 +21,6 @@ import { _t } from "../../languageHandler";
import { haveRendererForEvent } from "../../events/EventTileFactory"; import { haveRendererForEvent } from "../../events/EventTileFactory";
import SearchResultTile from "../views/rooms/SearchResultTile"; import SearchResultTile from "../views/rooms/SearchResultTile";
import { searchPagination, SearchScope } from "../../Searching"; import { searchPagination, SearchScope } from "../../Searching";
import Modal from "../../Modal";
import ErrorDialog from "../views/dialogs/ErrorDialog";
import type ResizeNotifier from "../../utils/ResizeNotifier"; import type ResizeNotifier from "../../utils/ResizeNotifier";
import MatrixClientContext from "../../contexts/MatrixClientContext"; import MatrixClientContext from "../../contexts/MatrixClientContext";
import { RoomPermalinkCreator } from "../../utils/permalinks/Permalinks"; import { RoomPermalinkCreator } from "../../utils/permalinks/Permalinks";
@ -45,7 +43,7 @@ interface Props {
abortController?: AbortController; abortController?: AbortController;
resizeNotifier: ResizeNotifier; resizeNotifier: ResizeNotifier;
className: string; className: string;
onUpdate(inProgress: boolean, results: ISearchResults | null): void; onUpdate(inProgress: boolean, results: ISearchResults | null, error: Error | null): void;
} }
// XXX: todo: merge overlapping results somehow? // XXX: todo: merge overlapping results somehow?
@ -70,7 +68,7 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
const handleSearchResult = useCallback( const handleSearchResult = useCallback(
(searchPromise: Promise<ISearchResults>): Promise<boolean> => { (searchPromise: Promise<ISearchResults>): Promise<boolean> => {
onUpdate(true, null); onUpdate(true, null, null);
return searchPromise.then( return searchPromise.then(
async (results): Promise<boolean> => { async (results): Promise<boolean> => {
@ -116,7 +114,7 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
setHighlights(highlights); setHighlights(highlights);
setResults({ ...results }); // copy to force a refresh setResults({ ...results }); // copy to force a refresh
onUpdate(false, results); onUpdate(false, results, null);
return false; return false;
}, },
(error) => { (error) => {
@ -125,11 +123,7 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
return false; return false;
} }
logger.error("Search failed", error); logger.error("Search failed", error);
Modal.createDialog(ErrorDialog, { onUpdate(false, null, error);
title: _t("error_dialog|search_failed|title"),
description: error?.message ?? _t("error_dialog|search_failed|server_unavailable"),
});
onUpdate(false, null);
return false; return false;
}, },
); );

View File

@ -1716,11 +1716,12 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
this.onSearch(this.state.search?.term ?? "", scope); this.onSearch(this.state.search?.term ?? "", scope);
}; };
private onSearchUpdate = (inProgress: boolean, searchResults: ISearchResults | null): void => { private onSearchUpdate = (inProgress: boolean, searchResults: ISearchResults | null, error: Error | null): void => {
this.setState({ this.setState({
search: { search: {
...this.state.search!, ...this.state.search!,
count: searchResults?.count, count: searchResults?.count,
error: error ?? undefined,
inProgress, inProgress,
}, },
}); });

View File

@ -40,6 +40,8 @@ const RoomSearchAuxPanel: React.FC<Props> = ({ searchInfo, isRoomEncrypted, onSe
{ count: searchInfo.count }, { count: searchInfo.count },
{ query: () => <strong>{searchInfo.term}</strong> }, { query: () => <strong>{searchInfo.term}</strong> },
) )
) : searchInfo?.error !== undefined ? (
searchInfo?.error.message
) : ( ) : (
<InlineSpinner /> <InlineSpinner />
)} )}

View File

@ -1133,11 +1133,7 @@
"title": "Unable to copy room link" "title": "Unable to copy room link"
}, },
"error_loading_user_profile": "Could not load user profile", "error_loading_user_profile": "Could not load user profile",
"forget_room_failed": "Failed to forget room %(errCode)s", "forget_room_failed": "Failed to forget room %(errCode)s"
"search_failed": {
"server_unavailable": "Server may be unavailable, overloaded, or search timed out :(",
"title": "Search failed"
}
}, },
"error_user_not_logged_in": "User is not logged in", "error_user_not_logged_in": "User is not logged in",
"event_preview": { "event_preview": {

View File

@ -247,7 +247,7 @@ describe("<RoomSearchView/>", () => {
await screen.findByRole("progressbar"); await screen.findByRole("progressbar");
await screen.findByText("Potato"); await screen.findByText("Potato");
expect(onUpdate).toHaveBeenCalledWith(false, expect.objectContaining({})); expect(onUpdate).toHaveBeenCalledWith(false, expect.objectContaining({}), null);
rerender( rerender(
<MatrixClientContext.Provider value={client}> <MatrixClientContext.Provider value={client}>
@ -314,7 +314,8 @@ describe("<RoomSearchView/>", () => {
}); });
}); });
it("should show modal if error is encountered", async () => { it("report error if one is encountered", async () => {
const onUpdate = jest.fn();
const deferred = defer<ISearchResults>(); const deferred = defer<ISearchResults>();
render( render(
@ -326,14 +327,18 @@ describe("<RoomSearchView/>", () => {
promise={deferred.promise} promise={deferred.promise}
resizeNotifier={resizeNotifier} resizeNotifier={resizeNotifier}
className="someClass" className="someClass"
onUpdate={jest.fn()} onUpdate={onUpdate}
/> />
</MatrixClientContext.Provider>, </MatrixClientContext.Provider>,
); );
deferred.reject(new Error("Some error")); deferred.reject("Some error");
try {
// Wait for RoomSearchView to process the promise
await deferred.promise;
} catch {}
await screen.findByText("Search failed"); expect(onUpdate).toHaveBeenCalledWith(false, null, "Some error");
await screen.findByText("Some error"); expect(onUpdate).toHaveBeenCalledTimes(2);
}); });
it("should combine search results when the query is present in multiple sucessive messages", async () => { it("should combine search results when the query is present in multiple sucessive messages", async () => {