From 88712994def77bf8940acff4a3f9efe99beb21b8 Mon Sep 17 00:00:00 2001 From: vj1235432 Date: Thu, 20 Aug 2026 00:48:54 +0400 Subject: [PATCH] fix(search): guard quick search against stale responses Quick search fired one request per keystroke with no AbortController and no staleness check, so whichever response landed last won regardless of which query it belonged to. Typing an address slowly showed hits belonging to a short prefix while the box held the full string; retyping the same string in one go correctly showed none, which reads as "search works sometimes". advancedSearch already guarded every write with controller.signal.aborted. This gives searchEmails the same treatment, reusing the existing searchAbortController so the two search paths supersede each other rather than race. The catch branch is guarded too: without it a superseded request that rejects clears results belonging to the current query and surfaces a misleading error. Adds stores/__tests__/email-store-search-race.test.ts with three cases, all of which fail on main and pass with this change. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/email-store-search-race.test.ts | 134 ++++++++++++++++++ stores/email-store.ts | 32 ++++- 2 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 stores/__tests__/email-store-search-race.test.ts diff --git a/stores/__tests__/email-store-search-race.test.ts b/stores/__tests__/email-store-search-race.test.ts new file mode 100644 index 000000000..38bb48937 --- /dev/null +++ b/stores/__tests__/email-store-search-race.test.ts @@ -0,0 +1,134 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore } from '../email-store'; +import { useAuthStore } from '../auth-store'; +import { useSettingsStore } from '@/stores/settings-store'; +import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +// Quick search fired an unguarded request per keystroke: no AbortController and +// no staleness check, so whichever response landed LAST won regardless of which +// query it belonged to. advancedSearch already guarded every write with +// `controller.signal.aborted`; searchEmails did not. +// +// The visible symptom is nasty because it looks like a server bug: type an +// address slowly and a short-prefix response arrives after the full-query one +// and repaints the list, so the box and the results disagree. Retype the same +// string in one go and you get the (correct) empty result, which reads as "the +// same search works sometimes and not others". + +function makeMailbox(overrides: Partial = {}): Mailbox { + return { + id: 'inbox', + name: 'Inbox', + sortOrder: 0, + totalEmails: 0, + unreadEmails: 0, + totalThreads: 0, + unreadThreads: 0, + isSubscribed: true, + isShared: false, + ...overrides, + } as Mailbox; +} + +function makeEmail(id: string): Email { + return { id, subject: id, receivedAt: '2026-01-01T00:00:00Z', mailboxIds: { inbox: true } } as unknown as Email; +} + +describe('quick search staleness', () => { + let client: IJMAPClient; + type Pending = { + resolve: (value: { emails: Email[]; hasMore: boolean; total: number }) => void; + reject: (reason: unknown) => void; + }; + let pending: Pending[]; + + beforeEach(() => { + pending = []; + + client = { + // Hand back a promise per call that the test settles by hand, so the + // responses can be made to arrive out of order. + searchEmails: vi.fn().mockImplementation( + () => new Promise((resolve, reject) => { pending.push({ resolve, reject }); }) + ), + advancedSearchEmails: vi.fn().mockResolvedValue({ emails: [], hasMore: false, total: 0 }), + getSomeEmails: vi.fn().mockResolvedValue([]), + } as unknown as IJMAPClient; + + useAuthStore.setState({ + activeAccountId: 'account-a', + getClientForAccount: (id: string) => (id === 'account-a' ? client : undefined) as never, + } as never); + + useSettingsStore.setState({ emailsPerPage: 50 } as never); + + useEmailStore.setState({ + isUnifiedView: false, + unifiedRole: null, + crossView: null, + viewingAccountId: null, + selectedMailbox: 'inbox', + searchMailboxId: '', + searchQuery: '', + searchFilters: { ...DEFAULT_SEARCH_FILTERS }, + searchAbortController: null, + emails: [], + mailboxes: [makeMailbox({ id: 'inbox', role: 'inbox' })], + accountMailboxes: {}, + }); + }); + + it('discards a slow response once a newer query has been started', async () => { + // Keystroke 1 goes out and stalls. + const first = useEmailStore.getState().searchEmails(client, 'qeti'); + // Keystroke 2 goes out before the first came back. + const second = useEmailStore.getState().searchEmails(client, 'qeti@smartchoice.ge'); + + expect(client.searchEmails).toHaveBeenCalledTimes(2); + + // The newer query legitimately matches nothing. + pending[1].resolve({ emails: [], hasMore: false, total: 0 }); + await second; + + // The older, broader query now answers late with hits. + pending[0].resolve({ emails: [makeEmail('stale-1'), makeEmail('stale-2')], hasMore: false, total: 2 }); + await first; + + const state = useEmailStore.getState(); + expect(state.searchQuery).toBe('qeti@smartchoice.ge'); + // Without the guard the late response repaints the list and the user sees + // two results for a query that matched none. + expect(state.emails).toEqual([]); + expect(state.totalEmails).toBe(0); + }); + + it('aborts the in-flight request when a new search starts', async () => { + const first = useEmailStore.getState().searchEmails(client, 'qeti'); + const controllerForFirst = useEmailStore.getState().searchAbortController; + expect(controllerForFirst?.signal.aborted).toBe(false); + + const second = useEmailStore.getState().searchEmails(client, 'qeti@smartchoice.ge'); + expect(controllerForFirst?.signal.aborted).toBe(true); + + pending.forEach(p => p.resolve({ emails: [], hasMore: false, total: 0 })); + await Promise.all([first, second]); + }); + + it('does not let a stale failure wipe the current results', async () => { + const first = useEmailStore.getState().searchEmails(client, 'qeti'); + const second = useEmailStore.getState().searchEmails(client, 'qeti@smartchoice.ge'); + + pending[1].resolve({ emails: [makeEmail('good-1')], hasMore: false, total: 1 }); + await second; + + // The superseded request rejects after the fact. + pending[0].reject(new Error("network blip")); + await first; + + const state = useEmailStore.getState(); + expect(state.error).toBeNull(); + expect(state.emails.map(e => e.id)).toEqual(['good-1']); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index beb2a225e..66ebfe887 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -2259,7 +2259,22 @@ export const useEmailStore = create((set, get) => ({ }, searchEmails: async (client, query) => { - set({ isLoading: true, error: null, searchQuery: query, emails: [], hasMoreEmails: false, totalEmails: 0 }); // Clear emails for loading state + const { searchAbortController } = get(); + + if (searchAbortController) { + searchAbortController.abort(); + } + + const controller = new AbortController(); + set({ + isLoading: true, + error: null, + searchQuery: query, + emails: [], + hasMoreEmails: false, + totalEmails: 0, + searchAbortController: controller, + }); // Clear emails for loading state try { const { isUnifiedView, unifiedRole, crossView, searchMailboxId, searchFilters } = get(); const emailsPerPage = useSettingsStore.getState().emailsPerPage; @@ -2308,10 +2323,14 @@ export const useEmailStore = create((set, get) => ({ result.total += newEmails.length; } + if (controller.signal.aborted) return; + const externals = await emailHooks.onProvideSearchResults.transform([] as ExternalSearchResult[], { - query, - filters: searchFilters + query, + filters: searchFilters }); + + if (controller.signal.aborted) return; result.emails = await emailHooks.onEmailsFetched.transform(result.emails); set({ emails: annotateScheduledEmails(result.emails, get().scheduledSubmissionByEmailId), @@ -2319,16 +2338,19 @@ export const useEmailStore = create((set, get) => ({ hasMoreEmails: result.hasMore, totalEmails: result.total, isLoading: false, - ...(unifiedErrors ? { unifiedErrors } : {}) + searchAbortController: null, + ...(unifiedErrors ? { unifiedErrors } : {}) }); } catch (error) { + if (controller.signal.aborted) return; set({ error: error instanceof Error ? error.message : "Failed to search emails", isLoading: false, emails: [], externalSearchResults: [], hasMoreEmails: false, - totalEmails: 0 + totalEmails: 0, + searchAbortController: null, }); } },