fix: keep the contact list when the address book can't be read

This commit is contained in:
Linus Rath
2026-11-03 05:47:00 +01:00
parent 79ae1d51f8
commit 7bb8447743
4 changed files with 172 additions and 7 deletions
+23 -4
View File
@@ -7327,7 +7327,10 @@ export class JMAPClient implements IJMAPClient {
}
}
/** Throws when no account could answer, so a failure doesn't read as having no address books. */
async getAllAddressBooks(): Promise<AddressBook[]> {
let answered = 0;
let failure: unknown;
try {
const allBooks: AddressBook[] = [];
const primaryId = this.getContactsAccountId();
@@ -7353,17 +7356,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);
failure ??= error;
}
}
return allBooks;
if (answered > 0 || !failure) return allBooks;
} catch (error) {
console.error('Failed to fetch all address books:', error);
return this.getAddressBooks();
}
throw failure;
}
async createAddressBook(name: string): Promise<AddressBook> {
@@ -8230,6 +8239,8 @@ export class JMAPClient implements IJMAPClient {
], this.contactUsing());
const queryResult = response.methodResponses?.[0];
// A refused call isn't an empty address book: the caller keeps what it has.
if (queryResult?.[0] === "error") throw new Error(queryResult[1]?.description || queryResult[1]?.type || "ContactCard/query failed");
if (queryResult?.[0] !== "ContactCard/query") break;
const ids: string[] = queryResult[1].ids || [];
@@ -8252,8 +8263,10 @@ export class JMAPClient implements IJMAPClient {
["ContactCard/get", { accountId, ids: chunk }, "g"],
], this.contactUsing());
if (response.methodResponses?.[0]?.[0] === "ContactCard/get") {
const list = (response.methodResponses[0][1].list || []) as ContactCard[];
const getResult = response.methodResponses?.[0];
if (getResult?.[0] === "error") throw new Error(getResult[1]?.description || getResult[1]?.type || "ContactCard/get failed");
if (getResult?.[0] === "ContactCard/get") {
const list = (getResult[1].list || []) as ContactCard[];
allContacts.push(...list.map(contactFromWire));
}
}
@@ -8273,7 +8286,10 @@ export class JMAPClient implements IJMAPClient {
}
}
/** Throws when no account could answer, so a failure doesn't read as an empty address book. */
async getAllContacts(): Promise<ContactCard[]> {
let answered = 0;
let failure: unknown;
try {
const allContacts: ContactCard[] = [];
const primaryId = this.getContactsAccountId();
@@ -8297,16 +8313,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);
failure ??= error;
}
}
return allContacts;
if (answered > 0 || !failure) return allContacts;
} catch (error) {
console.error('Failed to fetch all contacts:', error);
return this.getContacts();
}
throw failure;
}
async getContact(contactId: string, accountId?: string): Promise<ContactCard | null> {
+113
View File
@@ -0,0 +1,113 @@
// @vitest-environment node
/**
* The real JMAPClient reading the store's answers: what the client's own code
* makes of them is the contract, not the store's idea of the shape.
*/
import { afterEach, describe, expect, it } from 'vitest';
import { JMAPClient } from '@/lib/jmap/client';
import { setOfflineHooks, type OfflineHooks } from '@/lib/jmap/offline-hooks';
import { DEFAULT_SCOPE } from '../area';
import { contactsArea } from '../contacts';
import { AccountEngine } from '../engine/account-engine';
import { JMAP_ACCOUNT } from './helpers/fake-jmap';
import { CONTACTS_SESSION, FakeContacts, SHARED_ACCOUNT } from './helpers/fake-contacts';
import { openMemoryStore } from './helpers/node-adapter';
afterEach(() => setOfflineHooks(null));
async function offlineClient(server: FakeContacts): Promise<{ client: JMAPClient; engine: AccountEngine }> {
const engine = new AccountEngine({
account: 'a@fake.test',
db: openMemoryStore(),
session: CONTACTS_SESSION,
transport: server,
areas: [contactsArea],
scope: DEFAULT_SCOPE,
sleep: async () => {},
});
await engine.requestSync();
engine.setOnline(false);
const hooks: Partial<OfflineHooks> = {
skipNetwork: () => true,
answer: (_client, calls) => engine.answer(calls),
record: () => {},
noteNetwork: () => {},
};
setOfflineHooks(hooks as OfflineHooks);
const client = JMAPClient.fromOfflineSession('https://fake.test', 'a@example.org', {
...CONTACTS_SESSION,
capabilities: { ...CONTACTS_SESSION.capabilities, 'urn:ietf:params:jmap:core': { maxObjectsInGet: 2 } },
});
return { client, engine };
}
function seeded(): FakeContacts {
const server = new FakeContacts();
for (const full of ['Anna Bauer', 'Bernd Graf', 'Carla Müller']) server.addCard({ name: { full } });
server.addBook({ id: 'teambook', name: 'Team' }, SHARED_ACCOUNT);
server.addCard({ id: 'c1', name: { full: 'Dora Team' }, addressBookIds: { teambook: true } }, SHARED_ACCOUNT);
return server;
}
describe('the client over the store', () => {
it('reports a store that has nothing yet as a failure, not as no contacts', async () => {
const engine = new AccountEngine({
account: 'a@fake.test', db: openMemoryStore(), session: CONTACTS_SESSION, transport: new FakeContacts(),
areas: [contactsArea], scope: DEFAULT_SCOPE, sleep: async () => {},
});
// Answered per call, with one more call the store can answer: the error isn't hidden by a null answer.
const perCall: Partial<OfflineHooks> = {
skipNetwork: () => true,
answer: async (_client, calls) => (await engine.answer(calls)) ?? {
methodResponses: calls.map(([, , id]) => ['error', { type: 'serverUnavailable' }, id]),
},
record: () => {},
noteNetwork: () => {},
};
setOfflineHooks(perCall as OfflineHooks);
const client = JMAPClient.fromOfflineSession('https://fake.test', 'a@example.org', CONTACTS_SESSION);
await expect(client.getAllContacts()).rejects.toThrow();
await expect(client.getAllAddressBooks()).rejects.toThrow();
});
it('lists all contacts and books across accounts, paging in its own batch size', async () => {
const { client } = await offlineClient(seeded());
const contacts = await client.getAllContacts();
expect(contacts.map((c) => c.name?.full).sort()).toEqual(['Anna Bauer', 'Bernd Graf', 'Carla Müller', 'Dora Team']);
const shared = contacts.find((c) => c.name?.full === 'Dora Team');
expect(shared).toMatchObject({ id: `${SHARED_ACCOUNT}:c1`, originalId: 'c1', isShared: true });
const books = await client.getAllAddressBooks();
expect(books.map((b) => b.id).sort()).toEqual(['book1', `${SHARED_ACCOUNT}: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']);
expect((await client.searchContacts('team')).map((c) => c.originalId)).toEqual(['c1']);
});
it('creates, edits and deletes through its own methods with the stand-in id', async () => {
const { client, engine } = await offlineClient(seeded());
const created = await client.createContact({ name: { full: 'Neue Person' }, emails: { e: { address: 'neu@example.org' } } });
expect(created.id).toBeTruthy();
expect(created.addressBookIds).toEqual({ book1: true });
await client.updateContact(created.id, { name: { full: 'Neue Person B' } });
expect((await client.getContact(created.id))?.name?.full).toBe('Neue Person B');
expect((await client.searchContacts('person b')).map((c) => c.id)).toEqual([created.id]);
await client.deleteContact(created.id);
expect(await client.getContact(created.id)).toBeNull();
expect(await engine.areaCall('contacts.pending', undefined)).toBe(0);
});
it('throws the server-style error for an edit to a card it does not have', async () => {
const { client } = await offlineClient(seeded());
await expect(client.updateContact('nope', { name: { full: 'X' } })).rejects.toThrow();
await expect(client.deleteContact('nope')).rejects.toThrow();
});
it('reads an account the store has as the primary contacts account', async () => {
const { client } = await offlineClient(seeded());
expect(client.getContactsAccountId()).toBe(JMAP_ACCOUNT);
expect((await client.getContacts('book1')).length).toBe(3);
});
});
+33 -1
View File
@@ -9,7 +9,7 @@ import {
normalizeContactPhotoUri,
TRUSTED_SENDERS_BOOK_NAME,
} from '../contact-store';
import type { ContactCard, ContactMedia } from '@/lib/jmap/types';
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';
@@ -1364,6 +1364,38 @@ describe('contact-store', () => {
});
});
describe('an account that cannot answer', () => {
it('keeps its contacts and books while the other accounts refresh', async () => {
const failing = {
getAllContacts: vi.fn().mockRejectedValue(new Error('offline')),
getAllAddressBooks: vi.fn().mockRejectedValue(new Error('offline')),
} as unknown as IJMAPClient;
const working = {
getAllContacts: vi.fn().mockResolvedValue([makeContact({ id: 'b1' })]),
getAllAddressBooks: vi.fn().mockResolvedValue([{ id: 'bookb', name: 'B' }]),
} as unknown as IJMAPClient;
const kept = makeContact({ id: 'acc-a::a1', localAccountId: 'acc-a' });
const keptBook = { id: 'acc-a::booka', name: 'A', localAccountId: 'acc-a' } as AddressBook;
useContactStore.setState({ contacts: [kept], addressBooks: [keptBook] });
const accounts = [{ client: failing, localAccountId: 'acc-a' }, { client: working, localAccountId: 'acc-b' }];
await useContactStore.getState().fetchAllAccountsContacts(accounts);
await useContactStore.getState().fetchAllAccountsAddressBooks(accounts);
const state = useContactStore.getState();
expect(state.contacts.map((c) => c.id)).toEqual(['acc-a::a1', 'acc-b::b1']);
expect(state.addressBooks.map((b) => b.id)).toEqual(['acc-a::booka', 'acc-b::bookb']);
});
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()] });
await useContactStore.getState().fetchContacts(failing);
expect(useContactStore.getState().contacts).toHaveLength(1);
expect(useContactStore.getState().error).toBe('Failed to fetch contacts');
});
});
describe('persistence/partialize', () => {
it('should persist contacts when supportsSync is false', () => {
const { partialize } = (useContactStore as unknown as { persist: { getOptions: () => { partialize: (state: Record<string, unknown>) => Record<string, unknown> } } }).persist.getOptions();
+3 -2
View File
@@ -671,7 +671,8 @@ export const useContactStore = create<ContactStore>()(
return prefixContactsWithLocalAccount(list, localAccountId);
} catch (error) {
debug.error(`Failed to fetch contacts for account ${localAccountId}:`, error);
return [] as ContactCard[];
// Keep what this account's cards were: it just couldn't answer.
return get().contacts.filter((contact) => contact.localAccountId === localAccountId);
}
}),
);
@@ -694,7 +695,7 @@ export const useContactStore = create<ContactStore>()(
return prefixAddressBooksWithLocalAccount(list, localAccountId);
} catch (error) {
debug.error(`Failed to fetch address books for account ${localAccountId}:`, error);
return [] as AddressBook[];
return get().addressBooks.filter((book) => book.localAccountId === localAccountId);
}
}),
);