fix: keep calendars visible after a failed calendar fetch

This commit is contained in:
Linus Rath
2026-09-29 23:59:03 +02:00
parent 9d140fa762
commit 040cea30c5
6 changed files with 348 additions and 46 deletions
@@ -0,0 +1,114 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { JMAPClient } from '../jmap/client';
// The calendar store reconciles its persisted selection against the calendars
// list. getAllCalendars() swallowed every failure into an empty (or partial)
// list, which the store read as "these calendars are gone" and dropped from
// the selection for good. getAllCalendarsWithFailures() lets it tell the two
// apart.
function makeSession() {
return {
capabilities: { 'urn:ietf:params:jmap:core': {}, 'urn:ietf:params:jmap:calendars': {} },
accounts: {
'acct-1': { name: 'test', isPersonal: true, accountCapabilities: { 'urn:ietf:params:jmap:calendars': {} } },
// Shared accounts are probed on suspicion because they are non-personal.
'team': { name: 'Team', isPersonal: false, accountCapabilities: {} },
'ev': { name: 'No access', isPersonal: false, accountCapabilities: {} },
},
primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1', 'urn:ietf:params:jmap:calendars': 'acct-1' },
apiUrl: 'https://mail.example.com/jmap/api',
downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}',
uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/',
eventSourceUrl: '',
};
}
function jsonResponse(body: unknown): Response {
return new Response(JSON.stringify(body), { status: 200, headers: { 'Content-Type': 'application/json' } });
}
type Answer = 'ok' | 'serverFail' | 'network';
describe('getAllCalendarsWithFailures', () => {
let fetchSpy: ReturnType<typeof vi.spyOn>;
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
let client: JMAPClient;
let answers: Record<string, Answer>;
beforeEach(async () => {
fetchSpy = vi.spyOn(globalThis, 'fetch');
fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession()));
client = new JMAPClient('https://mail.example.com', 'user@test.com', 'pass123');
await client.connect();
fetchSpy.mockReset();
answers = { 'acct-1': 'ok', team: 'ok' };
fetchSpy.mockImplementation(async (_url: RequestInfo | URL, init?: RequestInit) => {
const body = JSON.parse(String(init?.body ?? '{}'));
const [method, args] = body.methodCalls?.[0] ?? [];
const accountId = args?.accountId as string;
if (accountId === 'ev') {
return jsonResponse({
methodResponses: [['error', { type: 'accountNotFound', description: 'You do not have access to account ev' }, '0']],
});
}
if (method === 'CalendarEvent/query') {
return jsonResponse({ methodResponses: [['CalendarEvent/query', { ids: [] }, '0']] });
}
const answer = answers[accountId];
if (answer === 'network') throw new TypeError('Failed to fetch');
if (answer === 'serverFail') {
return jsonResponse({ methodResponses: [['error', { type: 'serverFail', description: 'Try again' }, '0']] });
}
return jsonResponse({ methodResponses: [['Calendar/get', { list: [{ id: 'b', name: `${accountId} cal` }] }, '0']] });
});
consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(() => {
client.disconnect();
consoleErrorSpy.mockRestore();
fetchSpy.mockRestore();
});
it('lists every account and reports nothing failed when all load', async () => {
const { calendars, failedAccountIds } = await client.getAllCalendarsWithFailures();
expect(calendars.map((c) => c.id)).toEqual(['b', 'team:b']);
expect(failedAccountIds).toEqual([]);
});
it('throws when the primary account answers with a method error', async () => {
answers['acct-1'] = 'serverFail';
await expect(client.getAllCalendarsWithFailures()).rejects.toThrow('Try again');
});
it('throws when the primary account cannot be reached', async () => {
answers['acct-1'] = 'network';
await expect(client.getAllCalendarsWithFailures()).rejects.toThrow();
});
it('names a shared account that failed instead of silently leaving it out', async () => {
answers.team = 'serverFail';
const { calendars, failedAccountIds } = await client.getAllCalendarsWithFailures();
expect(calendars.map((c) => c.id)).toEqual(['b']);
expect(failedAccountIds).toEqual(['team']);
});
it('does not count a shared account without calendar access as failed', async () => {
const { failedAccountIds } = await client.getAllCalendarsWithFailures();
expect(failedAccountIds).not.toContain('ev');
});
it('keeps getAllCalendars lenient for callers that only display the list', async () => {
answers['acct-1'] = 'serverFail';
await expect(client.getAllCalendars()).resolves.toEqual([]);
});
});
+3
View File
@@ -884,6 +884,9 @@ export class DemoJMAPClient implements IJMAPClient {
async getCalendars(): Promise<Calendar[]> { return [...this.data.calendars]; }
async getAllCalendars(): Promise<Calendar[]> { return [...this.data.calendars]; }
async getAllCalendarsWithFailures(): Promise<{ calendars: Calendar[]; failedAccountIds: string[] }> {
return { calendars: [...this.data.calendars], failedAccountIds: [] };
}
async createCalendar(calendar: Partial<Calendar>): Promise<Calendar> {
const full: Calendar = {
+1
View File
@@ -430,6 +430,7 @@ export interface IJMAPClient {
getCalendarsAccountId(): string;
getCalendars(): Promise<Calendar[]>;
getAllCalendars(): Promise<Calendar[]>;
getAllCalendarsWithFailures(): Promise<{ calendars: Calendar[]; failedAccountIds: string[] }>;
createCalendar(calendar: Partial<Calendar>, targetAccountId?: string, options?: CreateCalendarOptions): Promise<Calendar>;
updateCalendar(calendarId: string, updates: Partial<Calendar>, targetAccountId?: string): Promise<void>;
setDefaultCalendar(calendarId: string, targetAccountId?: string): Promise<void>;
+64 -35
View File
@@ -434,6 +434,15 @@ function isTaskObject(obj: { '@type'?: string; progress?: unknown; due?: unknown
return false;
}
/**
* Whether a calendar method error means the account grants this user no
* calendar access at all - the expected answer from a shared account the
* fan-out probed on suspicion (see getCalendarCapableAccountIds).
*/
function isCalendarAccessDenial(type: string | undefined, message: string | undefined): boolean {
return type === 'forbidden' || type === 'accountNotFound' || /not have access/i.test(message ?? '');
}
const CALENDAR_EVENT_PROPERTIES = [
'id',
'@type',
@@ -6379,44 +6388,65 @@ export class JMAPClient implements IJMAPClient {
async getAllCalendars(): Promise<Calendar[]> {
try {
const allCalendars: Calendar[] = [];
const primaryId = this.getCalendarsAccountId();
const accountIds = this.getCalendarCapableAccountIds();
for (const accountId of accountIds) {
const isPrimary = accountId === primaryId;
if (!isPrimary && this.calendarAccessDenied.has(accountId)) continue;
const account = this.accounts[accountId];
try {
const response = await this.requestCalendars(accountId);
if (response.methodResponses?.[0]?.[0] === "Calendar/get") {
const rawCalendars = (response.methodResponses[0][1].list || []) as Calendar[];
const tasksOnly = await this.getTasksOnlyCalendarIds(accountId, rawCalendars.map((c) => c.id));
const calendars = rawCalendars.map((cal) => ({
...cal,
id: isPrimary ? cal.id : `${accountId}:${cal.id}`,
originalId: cal.id,
accountId,
accountName: account?.name || (isPrimary ? this.username : accountId),
isShared: !isPrimary,
isTasksOnly: tasksOnly.has(cal.id),
}));
allCalendars.push(...calendars);
}
} catch (error) {
console.error(`Failed to fetch calendars for account ${accountId}:`, error);
}
}
return allCalendars;
return (await this.getAllCalendarsWithFailures()).calendars;
} catch (error) {
console.error('Failed to fetch all calendars:', error);
return this.getCalendars();
return [];
}
}
/**
* getAllCalendars() for callers that reconcile state against the list: a
* failure on the primary account throws instead of yielding a list without
* its calendars, and shared accounts that could not be loaded are named in
* `failedAccountIds`. Read as "no calendars", a failed fetch dropped every
* calendar from the persisted selection.
*/
async getAllCalendarsWithFailures(): Promise<{ calendars: Calendar[]; failedAccountIds: string[] }> {
const allCalendars: Calendar[] = [];
const failedAccountIds: string[] = [];
const primaryId = this.getCalendarsAccountId();
const accountIds = this.getCalendarCapableAccountIds();
for (const accountId of accountIds) {
const isPrimary = accountId === primaryId;
if (!isPrimary && this.calendarAccessDenied.has(accountId)) continue;
const account = this.accounts[accountId];
try {
const response = await this.requestCalendars(accountId);
const [method, result] = response.methodResponses?.[0] ?? [];
if (method !== "Calendar/get") {
// Shared accounts are probed on suspicion (see
// getCalendarCapableAccountIds); one that grants no calendar access
// has no calendars to lose.
if (!isPrimary && isCalendarAccessDenial(result?.type, result?.description)) continue;
throw new Error(result?.description || result?.type || "Calendar/get failed");
}
const rawCalendars = (result.list || []) as Calendar[];
const tasksOnly = await this.getTasksOnlyCalendarIds(accountId, rawCalendars.map((c) => c.id));
const calendars = rawCalendars.map((cal) => ({
...cal,
id: isPrimary ? cal.id : `${accountId}:${cal.id}`,
originalId: cal.id,
accountId,
accountName: account?.name || (isPrimary ? this.username : accountId),
isShared: !isPrimary,
isTasksOnly: tasksOnly.has(cal.id),
}));
allCalendars.push(...calendars);
} catch (error) {
if (isPrimary) throw error;
console.error(`Failed to fetch calendars for account ${accountId}:`, error);
failedAccountIds.push(accountId);
}
}
return { calendars: allCalendars, failedAccountIds };
}
async createCalendar(calendar: Partial<Calendar>, targetAccountId?: string, options?: CreateCalendarOptions): Promise<Calendar> {
const accountId = targetAccountId || this.getCalendarsAccountId();
@@ -6913,8 +6943,7 @@ export class JMAPClient implements IJMAPClient {
// calendar access at all. Remember the rejection and go quiet instead
// of re-probing - and re-logging - on every range change.
const type = (error as { jmapErrorType?: string } | null)?.jmapErrorType;
const denied = type === 'forbidden' || type === 'accountNotFound' ||
/not have access/i.test(error instanceof Error ? error.message : '');
const denied = isCalendarAccessDenial(type, error instanceof Error ? error.message : undefined);
if (targetAccountId && denied) {
this.calendarAccessDenied.add(targetAccountId);
debug.log('calendar', `No calendar access to account ${targetAccountId} - skipping it from now on`);
@@ -0,0 +1,117 @@
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { useCalendarStore, reconcileSelectedIds } from '../calendar-store';
import type { IJMAPClient } from '@/lib/jmap/client-interface';
import type { Calendar } from '@/lib/jmap/types';
import { BIRTHDAY_CALENDAR_ID } from '@/lib/birthday-calendar';
// The calendar selection is persisted and reconciled against every calendars
// fetch. A failed fetch used to come back as an empty list: every real
// calendar dropped out of the selection, the always-kept birthday calendar
// stopped the "nothing left -> show all" fallback, and the calendar stayed on
// "birthdays only" for good.
function cal(overrides: Partial<Calendar>): Calendar {
return {
id: 'c', name: 'Cal', color: null, sortOrder: 0, isSubscribed: true, isVisible: true,
isDefault: false, includeInAvailability: 'all', timeZone: null, shareWith: null,
myRights: {
mayReadFreeBusy: true, mayReadItems: true, mayWriteAll: true, mayWriteOwn: true,
mayUpdatePrivate: true, mayRSVP: true, mayShare: true, mayDelete: true,
},
...overrides,
} as Calendar;
}
function clientReturning(result: { calendars: Calendar[]; failedAccountIds?: string[] } | Error): IJMAPClient {
return {
getAllCalendarsWithFailures: vi.fn(async () => {
if (result instanceof Error) throw result;
return { calendars: result.calendars, failedAccountIds: result.failedAccountIds ?? [] };
}),
} as unknown as IJMAPClient;
}
const own = [cal({ id: 'a', accountId: 'me' }), cal({ id: 'b', accountId: 'me' })];
describe('calendar selection after a fetch', () => {
beforeEach(() => {
useCalendarStore.setState({ calendars: [], selectedCalendarIds: [], error: null });
});
it('leaves the list and the selection alone when the fetch fails', async () => {
useCalendarStore.setState({ calendars: own, selectedCalendarIds: ['a', BIRTHDAY_CALENDAR_ID] });
await useCalendarStore.getState().fetchCalendars(clientReturning(new Error('Network error')));
const state = useCalendarStore.getState();
expect(state.selectedCalendarIds).toEqual(['a', BIRTHDAY_CALENDAR_ID]);
expect(state.calendars).toBe(own);
expect(state.error).toBe('Failed to load calendars');
});
it('falls back to the default when only the birthday calendar is left', async () => {
// The state the bug left behind: it heals on the next fetch.
useCalendarStore.setState({ selectedCalendarIds: [BIRTHDAY_CALENDAR_ID] });
await useCalendarStore.getState().fetchCalendars(clientReturning({ calendars: own }));
expect(useCalendarStore.getState().selectedCalendarIds).toEqual(['a', 'b', BIRTHDAY_CALENDAR_ID]);
});
it('falls back to the default without adding the birthday calendar when it was not selected', async () => {
useCalendarStore.setState({ selectedCalendarIds: ['gone'] });
await useCalendarStore.getState().fetchCalendars(clientReturning({ calendars: own }));
expect(useCalendarStore.getState().selectedCalendarIds).toEqual(['a', 'b']);
});
it('keeps a partial selection that still resolves', async () => {
useCalendarStore.setState({ selectedCalendarIds: ['b', BIRTHDAY_CALENDAR_ID] });
await useCalendarStore.getState().fetchCalendars(clientReturning({ calendars: own }));
expect(useCalendarStore.getState().selectedCalendarIds).toEqual(['b', BIRTHDAY_CALENDAR_ID]);
});
it('keeps the selected calendars of a shared account that failed to load', async () => {
useCalendarStore.setState({ selectedCalendarIds: ['a', 'team:x', 'gone'] });
await useCalendarStore.getState().fetchCalendars(clientReturning({ calendars: own, failedAccountIds: ['team'] }));
expect(useCalendarStore.getState().selectedCalendarIds).toEqual(['a', 'team:x']);
});
it('keeps the selected calendars of an aggregated account that failed to load', async () => {
// Both accounts have a calendar with raw id "b"; B's must not be remapped
// onto A's while B is unavailable.
useCalendarStore.setState({ selectedCalendarIds: ['A@h::b', 'B@h::b', 'B@h::shared:s'] });
await useCalendarStore.getState().fetchAllAccountsCalendars([
{ localAccountId: 'A@h', client: clientReturning({ calendars: [cal({ id: 'b', accountId: 'a1' })] }) },
{ localAccountId: 'B@h', client: clientReturning(new Error('Network error')) },
]);
expect(useCalendarStore.getState().selectedCalendarIds).toEqual(['A@h::b', 'B@h::b', 'B@h::shared:s']);
});
it('keeps the selected calendars of a shared account behind an aggregated account', async () => {
useCalendarStore.setState({ selectedCalendarIds: ['A@h::b', 'A@h::team:x'] });
await useCalendarStore.getState().fetchAllAccountsCalendars([
{ localAccountId: 'A@h', client: clientReturning({ calendars: [cal({ id: 'b', accountId: 'a1' })], failedAccountIds: ['team'] }) },
]);
expect(useCalendarStore.getState().selectedCalendarIds).toEqual(['A@h::b', 'A@h::team:x']);
});
});
describe('reconcileSelectedIds with unloaded accounts', () => {
it('keeps ids under an unloaded prefix verbatim instead of remapping them by raw id', () => {
const calendars = [cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h' })];
expect(reconcileSelectedIds(['B@h::b'], calendars, ['B@h::'])).toEqual(['B@h::b']);
// Without the prefix the id is remapped onto A's calendar (the raw->
// namespaced migration path).
expect(reconcileSelectedIds(['B@h::b'], calendars)).toEqual(['A@h::b']);
});
});
+49 -11
View File
@@ -275,10 +275,15 @@ async function refetchAfterOccurrenceMutation(): Promise<void> {
* namespaced transition (aggregation switched on, an account switch, or a
* just-created calendar added with its raw id) changes the id string while the
* calendar is the same. Remap by raw identity so the selection survives instead
* of silently resetting to "all". Keeps BIRTHDAY_CALENDAR_ID; drops ids that
* resolve to no calendar.
* of silently resetting to "all". Keeps BIRTHDAY_CALENDAR_ID and ids starting
* with one of `unloadedPrefixes` (calendars of an account that failed to load
* are unknown, not deleted); drops ids that resolve to no calendar.
*/
export function reconcileSelectedIds(selectedCalendarIds: string[], calendars: Calendar[]): string[] {
export function reconcileSelectedIds(
selectedCalendarIds: string[],
calendars: Calendar[],
unloadedPrefixes: string[] = [],
): string[] {
const byStoreId = new Set(calendars.map((c) => c.id));
const rawToStoreId = new Map<string, string>();
for (const c of calendars) {
@@ -290,6 +295,8 @@ export function reconcileSelectedIds(selectedCalendarIds: string[], calendars: C
for (const id of selectedCalendarIds) {
let mapped: string | undefined;
if (id === BIRTHDAY_CALENDAR_ID || byStoreId.has(id)) mapped = id;
// Before the raw remap: another account may hold the same raw id.
else if (unloadedPrefixes.some((prefix) => id.startsWith(prefix))) mapped = id;
else mapped = rawToStoreId.get(rawIdentityOf(id));
if (mapped && !seen.has(mapped)) {
out.push(mapped);
@@ -299,6 +306,23 @@ export function reconcileSelectedIds(selectedCalendarIds: string[], calendars: C
return out;
}
/**
* The selection to keep after a calendars fetch: the reconciled one, or the
* default when no real calendar is left in it. The virtual birthday calendar
* always survives reconciliation, so it must not count - counting it pinned
* the selection to "birthdays only" once the real calendars had dropped out.
*/
function selectionAfterFetch(
selectedCalendarIds: string[],
calendars: Calendar[],
defaultSelected: string[],
unloadedPrefixes?: string[],
): string[] {
const reconciled = reconcileSelectedIds(selectedCalendarIds, calendars, unloadedPrefixes);
if (reconciled.some((id) => id !== BIRTHDAY_CALENDAR_ID)) return reconciled;
return reconciled.includes(BIRTHDAY_CALENDAR_ID) ? [...defaultSelected, BIRTHDAY_CALENDAR_ID] : defaultSelected;
}
// In-flight refresh dedup. Concurrent callers (auto-interval +
// manual refresh, two account-switch reloads, etc.) share the same
// promise instead of double-fetching and racing the diff/import phase.
@@ -626,16 +650,22 @@ export const useCalendarStore = create<CalendarStore>()(
// alongside without blocking the grid.
void get().fetchParticipantIdentities(client);
try {
const calendars = await client.getAllCalendars();
const { selectedCalendarIds } = get();
const stillValid = reconcileSelectedIds(selectedCalendarIds, calendars);
// Throws when the primary account fails, so a failed fetch leaves the
// list and the selection as they were.
const { calendars, failedAccountIds } = await client.getAllCalendarsWithFailures();
// Default the visible selection to event calendars only - tasks-only
// calendars stay out of the event grid.
const defaultSelected = calendars.filter(c => !c.isTasksOnly).map(c => c.id);
set({
calendars,
isLoading: false,
selectedCalendarIds: stillValid.length > 0 ? stillValid : defaultSelected,
selectedCalendarIds: selectionAfterFetch(
get().selectedCalendarIds,
calendars,
defaultSelected,
// Shared calendars' ids are `<accountId>:<id>` (getAllCalendars).
failedAccountIds.map((accountId) => `${accountId}:`),
),
});
} catch (error) {
debug.error('Failed to fetch calendars:', error);
@@ -684,24 +714,32 @@ export const useCalendarStore = create<CalendarStore>()(
fetchAllAccountsCalendars: async (accounts) => {
set({ isLoading: true, error: null });
try {
// Id prefixes of the calendars that could not be loaded this time.
const unloadedPrefixes: string[] = [];
const results = await Promise.all(
accounts.map(async ({ client, localAccountId }) => {
const prefix = buildCrossAccountIdPrefix(localAccountId);
try {
const list = await client.getAllCalendars();
const { calendars: list, failedAccountIds } = await client.getAllCalendarsWithFailures();
unloadedPrefixes.push(...failedAccountIds.map((accountId) => `${prefix}${accountId}:`));
return prefixCalendarsWithLocalAccount(list, localAccountId);
} catch (error) {
debug.error(`Failed to fetch calendars for account ${localAccountId}:`, error);
unloadedPrefixes.push(prefix);
return [] as Calendar[];
}
}),
);
const calendars = results.flat();
const { selectedCalendarIds } = get();
const stillValid = reconcileSelectedIds(selectedCalendarIds, calendars);
set({
calendars,
isLoading: false,
selectedCalendarIds: stillValid.length > 0 ? stillValid : calendars.map(c => c.id),
selectedCalendarIds: selectionAfterFetch(
get().selectedCalendarIds,
calendars,
calendars.map(c => c.id),
unloadedPrefixes,
),
});
} catch (error) {
debug.error('Failed to fetch all-account calendars:', error);