fix(SEC-REVIEW-1): Surface search errors in LinkAutocomplete
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Previously the catch block in searchEntries silently swallowed all non-abort errors, showing "No entries found" when the search actually failed. This misled users into thinking the knowledge base was empty. - Add searchError state variable - Set PDA-friendly error message on non-abort failures - Clear error state on subsequent successful searches - Render error in amber (distinct from gray "No entries found") - Add 3 tests: error display, error clearing, abort exclusion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -49,6 +49,7 @@ export function LinkAutocomplete({
|
|||||||
const [results, setResults] = useState<SearchResult[]>([]);
|
const [results, setResults] = useState<SearchResult[]>([]);
|
||||||
const [selectedIndex, setSelectedIndex] = useState<number>(0);
|
const [selectedIndex, setSelectedIndex] = useState<number>(0);
|
||||||
const [isLoading, setIsLoading] = useState<boolean>(false);
|
const [isLoading, setIsLoading] = useState<boolean>(false);
|
||||||
|
const [searchError, setSearchError] = useState<string | null>(null);
|
||||||
const dropdownRef = useRef<HTMLDivElement>(null);
|
const dropdownRef = useRef<HTMLDivElement>(null);
|
||||||
const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null);
|
const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null);
|
||||||
const abortControllerRef = useRef<AbortController | null>(null);
|
const abortControllerRef = useRef<AbortController | null>(null);
|
||||||
@@ -88,6 +89,7 @@ export function LinkAutocomplete({
|
|||||||
|
|
||||||
setResults(searchResults);
|
setResults(searchResults);
|
||||||
setSelectedIndex(0);
|
setSelectedIndex(0);
|
||||||
|
setSearchError(null);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
// Ignore aborted requests - a newer search has superseded this one
|
// Ignore aborted requests - a newer search has superseded this one
|
||||||
if (error instanceof DOMException && error.name === "AbortError") {
|
if (error instanceof DOMException && error.name === "AbortError") {
|
||||||
@@ -95,6 +97,7 @@ export function LinkAutocomplete({
|
|||||||
}
|
}
|
||||||
console.error("Failed to search entries:", error);
|
console.error("Failed to search entries:", error);
|
||||||
setResults([]);
|
setResults([]);
|
||||||
|
setSearchError("Search unavailable — please try again");
|
||||||
} finally {
|
} finally {
|
||||||
if (!signal.aborted) {
|
if (!signal.aborted) {
|
||||||
setIsLoading(false);
|
setIsLoading(false);
|
||||||
@@ -371,6 +374,8 @@ export function LinkAutocomplete({
|
|||||||
>
|
>
|
||||||
{isLoading ? (
|
{isLoading ? (
|
||||||
<div className="p-3 text-sm text-gray-500 dark:text-gray-400">Searching...</div>
|
<div className="p-3 text-sm text-gray-500 dark:text-gray-400">Searching...</div>
|
||||||
|
) : searchError ? (
|
||||||
|
<div className="p-3 text-sm text-amber-600 dark:text-amber-400">{searchError}</div>
|
||||||
) : results.length === 0 ? (
|
) : results.length === 0 ? (
|
||||||
<div className="p-3 text-sm text-gray-500 dark:text-gray-400">
|
<div className="p-3 text-sm text-gray-500 dark:text-gray-400">
|
||||||
{state.query ? "No entries found" : "Start typing to search..."}
|
{state.query ? "No entries found" : "Start typing to search..."}
|
||||||
|
|||||||
@@ -207,6 +207,151 @@ describe("LinkAutocomplete", (): void => {
|
|||||||
vi.useRealTimers();
|
vi.useRealTimers();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("should show error message when search fails", async (): Promise<void> => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
|
||||||
|
mockApiRequest.mockRejectedValue(new Error("Network error"));
|
||||||
|
|
||||||
|
render(<LinkAutocomplete textareaRef={textareaRef} onInsert={onInsertMock} />);
|
||||||
|
|
||||||
|
const textarea = textareaRef.current;
|
||||||
|
if (!textarea) throw new Error("Textarea not found");
|
||||||
|
|
||||||
|
// Simulate typing [[fail
|
||||||
|
act(() => {
|
||||||
|
textarea.value = "[[fail";
|
||||||
|
textarea.setSelectionRange(6, 6);
|
||||||
|
fireEvent.input(textarea);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Advance past debounce to fire the search
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(300);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Allow microtasks (promise rejection handler) to settle
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Should show PDA-friendly error message instead of "No entries found"
|
||||||
|
expect(screen.getByText("Search unavailable — please try again")).toBeInTheDocument();
|
||||||
|
|
||||||
|
// Verify "No entries found" is NOT shown (error takes precedence)
|
||||||
|
expect(screen.queryByText("No entries found")).not.toBeInTheDocument();
|
||||||
|
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should clear error message on successful search", async (): Promise<void> => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
|
||||||
|
// First search fails
|
||||||
|
mockApiRequest.mockRejectedValueOnce(new Error("Network error"));
|
||||||
|
|
||||||
|
render(<LinkAutocomplete textareaRef={textareaRef} onInsert={onInsertMock} />);
|
||||||
|
|
||||||
|
const textarea = textareaRef.current;
|
||||||
|
if (!textarea) throw new Error("Textarea not found");
|
||||||
|
|
||||||
|
// Trigger failing search
|
||||||
|
act(() => {
|
||||||
|
textarea.value = "[[fail";
|
||||||
|
textarea.setSelectionRange(6, 6);
|
||||||
|
fireEvent.input(textarea);
|
||||||
|
});
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(300);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Allow microtasks (promise rejection handler) to settle
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(screen.getByText("Search unavailable — please try again")).toBeInTheDocument();
|
||||||
|
|
||||||
|
// Second search succeeds
|
||||||
|
mockApiRequest.mockResolvedValueOnce({
|
||||||
|
data: [
|
||||||
|
{
|
||||||
|
id: "1",
|
||||||
|
slug: "test-entry",
|
||||||
|
title: "Test Entry",
|
||||||
|
summary: "A test entry",
|
||||||
|
workspaceId: "workspace-1",
|
||||||
|
content: "Content",
|
||||||
|
contentHtml: "<p>Content</p>",
|
||||||
|
status: "PUBLISHED" as const,
|
||||||
|
visibility: "PUBLIC" as const,
|
||||||
|
createdBy: "user-1",
|
||||||
|
updatedBy: "user-1",
|
||||||
|
createdAt: new Date(),
|
||||||
|
updatedAt: new Date(),
|
||||||
|
tags: [],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
meta: { total: 1, page: 1, limit: 10, totalPages: 1 },
|
||||||
|
});
|
||||||
|
|
||||||
|
// Trigger successful search
|
||||||
|
act(() => {
|
||||||
|
textarea.value = "[[success";
|
||||||
|
textarea.setSelectionRange(9, 9);
|
||||||
|
fireEvent.input(textarea);
|
||||||
|
});
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(300);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Allow microtasks (promise resolution handler) to settle
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Error message should be gone, results should show
|
||||||
|
expect(screen.queryByText("Search unavailable — please try again")).not.toBeInTheDocument();
|
||||||
|
expect(screen.getByText("Test Entry")).toBeInTheDocument();
|
||||||
|
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not show error for aborted requests", async (): Promise<void> => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
|
||||||
|
// Make the API reject with an AbortError
|
||||||
|
const abortError = new DOMException("The operation was aborted.", "AbortError");
|
||||||
|
mockApiRequest.mockRejectedValue(abortError);
|
||||||
|
|
||||||
|
render(<LinkAutocomplete textareaRef={textareaRef} onInsert={onInsertMock} />);
|
||||||
|
|
||||||
|
const textarea = textareaRef.current;
|
||||||
|
if (!textarea) throw new Error("Textarea not found");
|
||||||
|
|
||||||
|
// Simulate typing [[abc
|
||||||
|
act(() => {
|
||||||
|
textarea.value = "[[abc";
|
||||||
|
textarea.setSelectionRange(5, 5);
|
||||||
|
fireEvent.input(textarea);
|
||||||
|
});
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(300);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Should NOT show error message for aborted requests
|
||||||
|
// Allow a tick for the catch to process
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(screen.queryByText("Search unavailable — please try again")).not.toBeInTheDocument();
|
||||||
|
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
// TODO: Fix async/timer interaction - component works but test has timing issues with fake timers
|
// TODO: Fix async/timer interaction - component works but test has timing issues with fake timers
|
||||||
it.skip("should perform debounced search when typing query", async (): Promise<void> => {
|
it.skip("should perform debounced search when typing query", async (): Promise<void> => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
|
|||||||
Reference in New Issue
Block a user