diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 56258dd86..af3a7e095 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -19,6 +19,7 @@ import { isTaskLikeObject, type ScannedCalendarObject } from "@/lib/calendar-com import { DEFAULT_CALENDAR_COMPONENTS, mkCalendarCollection, newCalendarCollectionName } from "@/lib/webdav/calendar-collection"; import { sanitizeDisplayName, splitMailbox } from "@/lib/rfc5322-mailbox"; import { decodeFileNodeName } from "./filenode-name"; +import { PartialAccountsError } from "./partial-accounts"; import { fileNodeKind, fromDraftFileNode, toDraftFileNodeCreate } from "./filenode-kind"; import { fileNodeDepth } from "./filenode-depth"; import { contactFromWire, contactToWire } from "./contact-wire"; @@ -7327,13 +7328,16 @@ export class JMAPClient implements IJMAPClient { } } - /** Throws when no account could answer, so a failure doesn't read as having no address books. */ + /** + * Throws PartialAccountsError when an account couldn't answer, with what the + * others gave, so a failure doesn't read as that account having no books. + */ async getAllAddressBooks(): Promise { - let answered = 0; + const failed: string[] = []; let failure: unknown; + const allBooks: AddressBook[] = []; + const primaryId = this.getContactsAccountId(); try { - const allBooks: AddressBook[] = []; - const primaryId = this.getContactsAccountId(); const accountIds = this.getContactCapableAccountIds(); for (const accountId of accountIds) { @@ -7356,23 +7360,23 @@ export class JMAPClient implements IJMAPClient { isShared: !isPrimary, })); allBooks.push(...books); - answered += 1; } else if (response.methodResponses?.[0]?.[0] === "error") { const refused = response.methodResponses[0][1]; throw new Error(refused?.description || refused?.type || "AddressBook/get failed"); } } catch (error) { console.error(`Failed to fetch address books for account ${accountId}:`, error); + failed.push(accountId); failure ??= error; } } - if (answered > 0 || !failure) return allBooks; + if (failed.length === 0) return allBooks; } catch (error) { console.error('Failed to fetch all address books:', error); return this.getAddressBooks(); } - throw failure; + throw new PartialAccountsError(allBooks, failed, primaryId, failure); } async createAddressBook(name: string): Promise { @@ -8286,13 +8290,17 @@ export class JMAPClient implements IJMAPClient { } } - /** Throws when no account could answer, so a failure doesn't read as an empty address book. */ + /** + * Throws PartialAccountsError when an account couldn't answer, with what the + * others gave, so a failure doesn't read as that account having no cards. + */ async getAllContacts(): Promise { - let answered = 0; + const failed: string[] = []; let failure: unknown; + let allContacts: ContactCard[] = []; + const primaryId = this.getContactsAccountId(); try { - const allContacts: ContactCard[] = []; - const primaryId = this.getContactsAccountId(); + allContacts = []; const accountIds = this.getContactCapableAccountIds(); for (const accountId of accountIds) { @@ -8313,19 +8321,19 @@ export class JMAPClient implements IJMAPClient { isShared: !isPrimary, })); allContacts.push(...contacts); - answered += 1; } catch (error) { console.error(`Failed to fetch contacts for account ${accountId}:`, error); + failed.push(accountId); failure ??= error; } } - if (answered > 0 || !failure) return allContacts; + if (failed.length === 0) return allContacts; } catch (error) { console.error('Failed to fetch all contacts:', error); return this.getContacts(); } - throw failure; + throw new PartialAccountsError(allContacts, failed, primaryId, failure); } async getContact(contactId: string, accountId?: string): Promise { diff --git a/lib/jmap/partial-accounts.ts b/lib/jmap/partial-accounts.ts new file mode 100644 index 000000000..66d3798a8 --- /dev/null +++ b/lib/jmap/partial-accounts.ts @@ -0,0 +1,27 @@ +/** + * A listing across JMAP accounts where at least one account couldn't answer. + * It carries what the others gave. An account that failed isn't an account + * with nothing in it, so the caller keeps what it had for that one. + */ +export class PartialAccountsError extends Error { + constructor( + readonly items: T[], + readonly failedAccountIds: string[], + readonly primaryAccountId: string, + cause: unknown, + ) { + super(cause instanceof Error ? cause.message : 'An account could not answer'); + this.name = 'PartialAccountsError'; + } +} + +/** By name: `instanceof` fails across Next's bundle copies. */ +export function partialAccounts(error: unknown): PartialAccountsError | null { + return error instanceof Error && error.name === 'PartialAccountsError' ? error as PartialAccountsError : null; +} + +/** What to show after a partial listing: the answers, and what's held for the accounts that failed. */ +export function withFailedAccountsKept(partial: PartialAccountsError, held: readonly T[]): T[] { + const failed = new Set(partial.failedAccountIds); + return [...partial.items, ...held.filter((item) => failed.has(item.accountId ?? partial.primaryAccountId))]; +} diff --git a/lib/store/__tests__/client-contacts.test.ts b/lib/store/__tests__/client-contacts.test.ts index 87d516a34..7e97c18f3 100644 --- a/lib/store/__tests__/client-contacts.test.ts +++ b/lib/store/__tests__/client-contacts.test.ts @@ -6,6 +6,8 @@ import { afterEach, describe, expect, it } from 'vitest'; import { JMAPClient } from '@/lib/jmap/client'; import { setOfflineHooks, type OfflineHooks } from '@/lib/jmap/offline-hooks'; +import { partialAccounts } from '@/lib/jmap/partial-accounts'; +import type { AddressBook, ContactCard } from '@/lib/jmap/types'; import { DEFAULT_SCOPE } from '../area'; import { contactsArea } from '../contacts'; import { AccountEngine } from '../engine/account-engine'; @@ -80,6 +82,27 @@ describe('the client over the store', () => { expect(books.map((b) => b.id).sort()).toEqual(['book1', `${SHARED_ACCOUNT}:teambook`]); }); + it('says which account failed, with what the others gave', async () => { + const { client, engine } = await offlineClient(seeded()); + const failPrimary: Partial = { + skipNetwork: () => true, + answer: async (_client, calls) => (calls[0][1].accountId === JMAP_ACCOUNT + ? { methodResponses: calls.map(([, , id]) => ['error', { type: 'serverUnavailable' }, id]) } + : engine.answer(calls)), + record: () => {}, + noteNetwork: () => {}, + }; + setOfflineHooks(failPrimary as OfflineHooks); + const contacts = await client.getAllContacts().catch((error: unknown) => error); + const partial = partialAccounts(contacts); + expect(partial?.failedAccountIds).toEqual([JMAP_ACCOUNT]); + expect(partial?.primaryAccountId).toBe(JMAP_ACCOUNT); + expect(partial?.items.map((c) => c.name?.full)).toEqual(['Dora Team']); + const books = partialAccounts(await client.getAllAddressBooks().catch((error: unknown) => error)); + expect(books?.failedAccountIds).toEqual([JMAP_ACCOUNT]); + expect(books?.items.map((b) => b.originalId)).toEqual(['teambook']); + }); + it('searches with its own filter', async () => { const { client } = await offlineClient(seeded()); expect((await client.searchContacts('müll')).map((c) => c.name?.full)).toEqual(['Carla Müller']); diff --git a/stores/__tests__/contact-store.test.ts b/stores/__tests__/contact-store.test.ts index f9dfabcca..f52aa4e11 100644 --- a/stores/__tests__/contact-store.test.ts +++ b/stores/__tests__/contact-store.test.ts @@ -13,6 +13,7 @@ import type { AddressBook, ContactCard, ContactMedia } from '@/lib/jmap/types'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; import { clearContactPhotoCache } from '@/lib/contact-photo-cache'; import { PrincipalsRequestError } from '@/lib/jmap/principals-error'; +import { PartialAccountsError } from '@/lib/jmap/partial-accounts'; import { setClientLookup } from '../client-registry'; vi.stubGlobal('crypto', { randomUUID: () => '00000000-0000-0000-0000-000000000000' }); @@ -1387,6 +1388,46 @@ describe('contact-store', () => { expect(state.addressBooks.map((b) => b.id)).toEqual(['acc-a::booka', 'acc-b::bookb']); }); + it('keeps the primary account\'s cards when only a shared account answered', async () => { + const refusal = new PartialAccountsError([makeContact({ id: 'shared:s2', accountId: 'shared', isShared: true })], ['me'], 'me', new Error('offline')); + const failing = { getAllContacts: vi.fn().mockRejectedValue(refusal) } as unknown as IJMAPClient; + useContactStore.setState({ + contacts: [ + makeContact({ id: 'p1', accountId: 'me' }), + // A card just created has no accountId yet: it's the primary account's. + makeContact({ id: 'p2' }), + makeContact({ id: 'shared:s1', accountId: 'shared', isShared: true }), + ], + }); + await useContactStore.getState().fetchContacts(failing); + const state = useContactStore.getState(); + expect(state.contacts.map((c) => c.id).sort()).toEqual(['p1', 'p2', 'shared:s2']); + expect(state.error).toBe('Failed to fetch contacts'); + expect(state.isLoading).toBe(false); + }); + + it('keeps the primary account\'s books when only a shared account answered', async () => { + const refusal = new PartialAccountsError([{ id: 'shared:b', name: 'B', accountId: 'shared' } as AddressBook], ['me'], 'me', new Error('offline')); + const failing = { getAllAddressBooks: vi.fn().mockRejectedValue(refusal) } as unknown as IJMAPClient; + useContactStore.setState({ addressBooks: [{ id: 'mine', name: 'Mine', accountId: 'me' } as AddressBook] }); + await useContactStore.getState().fetchAddressBooks(failing); + expect(useContactStore.getState().addressBooks.map((b) => b.id).sort()).toEqual(['mine', 'shared:b']); + }); + + it('keeps a failed JMAP account of one login while its others refresh', async () => { + const refusal = new PartialAccountsError([makeContact({ id: 's2', accountId: 'shared' })], ['me'], 'me', new Error('offline')); + const failing = { getAllContacts: vi.fn().mockRejectedValue(refusal) } as unknown as IJMAPClient; + useContactStore.setState({ + contacts: [ + makeContact({ id: 'acc-a::p1', accountId: 'me', localAccountId: 'acc-a' }), + makeContact({ id: 'acc-a::s1', accountId: 'shared', localAccountId: 'acc-a' }), + makeContact({ id: 'acc-b::x', accountId: 'me', localAccountId: 'acc-b' }), + ], + }); + await useContactStore.getState().fetchAllAccountsContacts([{ client: failing, localAccountId: 'acc-a' }]); + expect(useContactStore.getState().contacts.map((c) => c.id).sort()).toEqual(['acc-a::p1', 'acc-a::s2']); + }); + it('keeps the list when the only account fails', async () => { const failing = { getAllContacts: vi.fn().mockRejectedValue(new Error('offline')) } as unknown as IJMAPClient; useContactStore.setState({ contacts: [makeContact()] }); diff --git a/stores/contact-store.ts b/stores/contact-store.ts index 3403088b2..924dfc99a 100644 --- a/stores/contact-store.ts +++ b/stores/contact-store.ts @@ -13,6 +13,7 @@ import { sanitizeDisplayName, splitMailbox } from '@/lib/rfc5322-mailbox'; import { inlineContactMedia, writeContactWithNewPhoto, type ContactSaveOptions } from '@/lib/contact-media'; import { contactToView } from '@/lib/plugin-projection'; import { PrincipalsRequestError } from '@/lib/jmap/principals-error'; +import { PartialAccountsError, partialAccounts, withFailedAccountsKept } from '@/lib/jmap/partial-accounts'; import type { Principal } from '@/lib/jmap/types'; import { currentStoreEpoch } from '@/lib/store-epoch'; @@ -614,7 +615,14 @@ export const useContactStore = create()( } catch (error) { if (currentStoreEpoch() !== epoch) return; console.error('Failed to fetch contacts:', error); - set({ error: 'Failed to fetch contacts', isLoading: false, contactsLoaded: true }); + // An account that couldn't answer keeps its cards; the others are fresh. + const partial = partialAccounts(error); + set((state) => ({ + error: 'Failed to fetch contacts', + isLoading: false, + contactsLoaded: true, + contacts: partial ? withFailedAccountsKept(partial, state.contacts) : state.contacts, + })); } }, @@ -656,7 +664,11 @@ export const useContactStore = create()( } catch (error) { if (currentStoreEpoch() !== epoch) return; console.error('Failed to fetch address books:', error); - set({ error: 'Failed to fetch address books' }); + const partial = partialAccounts(error); + set((state) => ({ + error: 'Failed to fetch address books', + addressBooks: partial ? withFailedAccountsKept(partial, state.addressBooks) : state.addressBooks, + })); } }, @@ -671,8 +683,12 @@ export const useContactStore = create()( return prefixContactsWithLocalAccount(list, localAccountId); } catch (error) { debug.error(`Failed to fetch contacts for account ${localAccountId}:`, error); - // Keep what this account's cards were: it just couldn't answer. - return get().contacts.filter((contact) => contact.localAccountId === localAccountId); + // Keep what this login's cards were: it just couldn't answer. + const held = get().contacts.filter((contact) => contact.localAccountId === localAccountId); + const partial = partialAccounts(error); + if (!partial) return held; + const fresh = prefixContactsWithLocalAccount(partial.items, localAccountId); + return withFailedAccountsKept(new PartialAccountsError(fresh, partial.failedAccountIds, partial.primaryAccountId, error), held); } }), ); @@ -695,7 +711,11 @@ export const useContactStore = create()( return prefixAddressBooksWithLocalAccount(list, localAccountId); } catch (error) { debug.error(`Failed to fetch address books for account ${localAccountId}:`, error); - return get().addressBooks.filter((book) => book.localAccountId === localAccountId); + const held = get().addressBooks.filter((book) => book.localAccountId === localAccountId); + const partial = partialAccounts(error); + if (!partial) return held; + const fresh = prefixAddressBooksWithLocalAccount(partial.items, localAccountId); + return withFailedAccountsKept(new PartialAccountsError(fresh, partial.failedAccountIds, partial.primaryAccountId, error), held); } }), );