From 47f2971899820bc1dbe17024e2f27f494b1275c2 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:50:15 +0100 Subject: [PATCH] fix: keep a subscription's events when its feed arrives cut off or drops most of them at once --- locales/rn/en.json | 2 + .../calendar-subscriptions-store.test.ts | 90 +++++++++++++++++++ src/stores/calendar-subscriptions-store.ts | 47 ++++++++-- 3 files changed, 131 insertions(+), 8 deletions(-) diff --git a/locales/rn/en.json b/locales/rn/en.json index 679fe44d..dc57ce5d 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -137,7 +137,9 @@ "account_changed": "The account changed while the subscription was syncing. It syncs again in its own account.", "add_feed": "Add a feed", "feed_too_large": "Feed is too large", + "feed_truncated": "The feed was cut off before its end. Nothing was changed.", "fetch_failed": "Could not fetch feed (HTTP {status})", + "mass_removal": "The feed left out {count} of the {total} events at once. They were kept in case the feed is broken.", "name_and_url_required": "Enter a name and a feed URL.", "not_a_feed": "That URL did not return an iCalendar feed", "not_synced": "Not synced yet", diff --git a/src/stores/__tests__/calendar-subscriptions-store.test.ts b/src/stores/__tests__/calendar-subscriptions-store.test.ts index 6eb7f5e3..3d6b8dac 100644 --- a/src/stores/__tests__/calendar-subscriptions-store.test.ts +++ b/src/stores/__tests__/calendar-subscriptions-store.test.ts @@ -23,9 +23,17 @@ const calendarStore = vi.hoisted(() => ({ fetchCalendars: vi.fn(async () => undefined), refresh: vi.fn(async () => undefined), })); +const { CalendarAccountChangedError } = vi.hoisted(() => ({ + CalendarAccountChangedError: class CalendarAccountChangedError extends Error { + constructor() { + super('The account changed while the calendar was loading or saving.'); + } + }, +})); vi.mock('../calendar-store', () => ({ useCalendarStore: { getState: () => calendarStore }, calendarStoreGeneration: () => calendarStore.generation, + CalendarAccountChangedError, })); vi.mock('@react-native-async-storage/async-storage', () => ({ default: { @@ -447,4 +455,86 @@ describe('refreshing a subscription', () => { expect(calendarApi.updateEvent).not.toHaveBeenCalled(); expect(calendarStore.importEvents).not.toHaveBeenCalled(); }); + + it('counts the calendar store stopping for an account switch as no failure of the subscription', async () => { + calendarStore.importEvents.mockRejectedValueOnce(new CalendarAccountChangedError()); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarStore.importEvents).toHaveBeenCalledTimes(1); + expect(only()).toMatchObject({ lastSyncAt: 5, lastError: null }); + }); + + // A body cut off mid-VEVENT parses up to the cut: every mirror event after + // it looked gone upstream and was deleted. + it('changes nothing when the feed was cut off before its end', async () => { + vi.mocked(globalThis.fetch).mockResolvedValueOnce({ + ok: true, status: 200, headers: { get: () => null }, + text: async () => 'BEGIN:VCALENDAR\r\nBEGIN:VEVENT\r\nUID:kept\r\nDTSTART:20270102T100000Z\r\nEND:VEVENT\r\nBEGIN:VEVENT\r\nUID:sa', + } as unknown as Response); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarApi.parseCalendarBlob).not.toHaveBeenCalled(); + expect(calendarApi.deleteEvents).not.toHaveBeenCalled(); + expect(calendarStore.importEvents).not.toHaveBeenCalled(); + expect(only().lastError).toMatch(/cut off/); + expect(only().lastSyncAt).toBe(5); + }); + + describe('a feed that leaves out much of a large mirror at once', () => { + const mirror = Array.from({ length: 10 }, (_, i) => ({ id: `m${i}`, uid: `u${i}`, calendarIds: { b: true } })); + const feedOf = (count: number) => mirror.slice(0, count).map((e) => ({ uid: e.uid, title: e.uid, start: '2027-01-02T10:00:00' })); + + beforeEach(() => { + vi.mocked(calendarApi.queryEvents).mockResolvedValue(mirror.map((e) => e.id)); + vi.mocked(calendarApi.getEventIdentities).mockResolvedValue(mirror as never); + }); + + it('keeps the events and says so when more than half of them left the feed', async () => { + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue(feedOf(4) as never); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarApi.deleteEvents).not.toHaveBeenCalled(); + expect(calendarApi.updateEvent).not.toHaveBeenCalled(); + expect(only().lastError).toMatch(/6 of the 10/); + }); + + it('drops them when half of them left', async () => { + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue(feedOf(5) as never); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarApi.deleteEvents).toHaveBeenCalledWith(['m5', 'm6', 'm7', 'm8', 'm9'], undefined, 'acc-1'); + expect(only().lastError).toBeNull(); + }); + }); +}); + +describe('adding a subscription', () => { + beforeEach(() => { + vi.clearAllMocks(); + client.accountId = 'acc-1'; + calendarStore.generation = 0; + useCalendarSubscriptionsStore.setState({ syncing: {}, subscriptions: [] }); + globalThis.fetch = vi.fn(async () => ({ + ok: true, status: 200, headers: { get: () => null }, text: async () => 'BEGIN:VCALENDAR\r\nEND:VCALENDAR\r\n', + })) as unknown as typeof fetch; + vi.mocked(calendarApi.createCalendar).mockResolvedValue({ id: 'n1', name: 'Holidays' } as never); + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue([{ uid: 'e1', title: 'New year', start: '2027-01-01T00:00:00' }] as never); + }); + + // The mirror is in the account the add started in: removing it then would + // reach the next account's calendar of the same id. + it('keeps the subscription and its mirror when the calendar store stopped for an account switch', async () => { + calendarStore.importEvents.mockRejectedValueOnce(new CalendarAccountChangedError()); + + await expect(useCalendarSubscriptionsStore.getState().addSubscription({ + name: 'Holidays', url: 'https://feeds.example/holidays.ics', + })).rejects.toBeInstanceOf(CalendarAccountChangedError); + + expect(calendarApi.deleteCalendar).not.toHaveBeenCalled(); + expect(useCalendarSubscriptionsStore.getState().subscriptions).toMatchObject([{ calendarId: 'n1', accountId: 'acc-1', lastError: null }]); + }); }); diff --git a/src/stores/calendar-subscriptions-store.ts b/src/stores/calendar-subscriptions-store.ts index fad85615..f5cc47b3 100644 --- a/src/stores/calendar-subscriptions-store.ts +++ b/src/stores/calendar-subscriptions-store.ts @@ -20,7 +20,7 @@ import { generateAccountId } from '../lib/account-utils'; import { feedChanged, feedFields } from '../lib/calendar-feed-diff'; import { readServerPrimaryAccountId } from '../lib/server-fingerprint'; import { useAccountStore } from './account-store'; -import { calendarStoreGeneration, useCalendarStore } from './calendar-store'; +import { CalendarAccountChangedError, calendarStoreGeneration, useCalendarStore } from './calendar-store'; import { t } from './locale-store'; export interface CalendarSubscription { @@ -53,6 +53,9 @@ export const DEFAULT_REFRESH_INTERVAL_MINUTES = 60; // 10 MB is far beyond any real feed; a bigger response is not a calendar. const MAX_FEED_BYTES = 10 * 1024 * 1024; +// A sync removes at most half of a mirror this size or larger at once. +const MASS_REMOVAL_MIN_EVENTS = 10; + interface SubscriptionsState { subscriptions: CalendarSubscription[]; syncing: Record; @@ -205,6 +208,14 @@ class SubscriptionAccountChangedError extends Error { } } +/** + * Whether `err` says the account changed under the sync: its own check, or + * the calendar store's (importEvents stops so after an account switch). + */ +function accountChanged(err: unknown): boolean { + return err instanceof SubscriptionAccountChangedError || err instanceof CalendarAccountChangedError; +} + function assertScope(scope: SyncScope): void { if (currentAccountId() !== scope.accountId || calendarStoreGeneration() !== scope.generation) { throw new SubscriptionAccountChangedError(); @@ -225,6 +236,11 @@ async function fetchAndParseFeed(url: string, scope: SyncScope): Promise { +): Promise { const { accountId } = scope; const parsed = await fetchAndParseFeed(url, scope); let toImport = parsed; let wrote = false; + let warning: string | null = null; if (linkedBefore) { const parsedUids = new Set(parsed.map((e) => e.uid).filter(Boolean) as string[]); @@ -269,7 +288,18 @@ async function syncFeedIntoCalendar( const inMirror = new Set(existing.map((e) => e.uid).filter(Boolean) as string[]); toImport = parsed.filter((e) => !e.uid || !inMirror.has(e.uid)); - const stale = existing.filter((e) => e.uid && !parsedUids.has(e.uid)); + let stale = existing.filter((e) => e.uid && !parsedUids.has(e.uid)); + // A feed that drops most of a sizeable mirror at once is more likely + // broken (an error page that still parses, a server's partial answer) + // than emptied: the events stay, and the sync says so. + if (existing.length >= MASS_REMOVAL_MIN_EVENTS && stale.length > existing.length / 2) { + warning = t( + 'calendar.subscription.mass_removal', + 'The feed left out {count} of the {total} events at once. They were kept in case the feed is broken.', + { count: stale.length, total: existing.length }, + ); + stale = []; + } const toDelete: string[] = []; for (const e of stale) { const others = { ...(e.calendarIds || {}) }; @@ -308,7 +338,7 @@ async function syncFeedIntoCalendar( } } } catch (err) { - if (err instanceof SubscriptionAccountChangedError) throw err; + if (accountChanged(err)) throw err; // Non-fatal: if we can't enumerate existing events, still import new ones. } } @@ -317,6 +347,7 @@ async function syncFeedIntoCalendar( // importEvents shows what it brought in; deletions and changes alone need a reload. if (toImport.length > 0) await useCalendarStore.getState().importEvents(toImport, calendarId); else if (wrote) await useCalendarStore.getState().refresh(); + return warning; } export const useCalendarSubscriptionsStore = create()( @@ -353,7 +384,7 @@ export const useCalendarSubscriptionsStore = create()( await useCalendarStore.getState().refresh(); return { ...sub, lastSyncAt: Date.now() }; } catch (err) { - if (err instanceof SubscriptionAccountChangedError) { + if (accountChanged(err)) { // The mirror is in the account the sync started in, which the // client no longer serves: a request to remove it would reach // the next account. Kept as that account's subscription, it @@ -473,17 +504,17 @@ export const useCalendarSubscriptionsStore = create()( await useCalendarStore.getState().fetchCalendars(); } const linkedBefore = calendarId === sub.calendarId; - await syncFeedIntoCalendar(calendarId, sub.url, linkedBefore, scope); + const warning = await syncFeedIntoCalendar(calendarId, sub.url, linkedBefore, scope); set({ subscriptions: get().subscriptions.map((s) => - s.id === id ? { ...s, lastSyncAt: Date.now(), lastError: null } : s, + s.id === id ? { ...s, lastSyncAt: Date.now(), lastError: warning } : s, ), }); if (!linkedBefore) await useCalendarStore.getState().refresh(); } catch (err) { // Not this subscription's failure: the client serves another // account now, and the sync runs again in its own. - if (err instanceof SubscriptionAccountChangedError) return; + if (accountChanged(err)) return; const message = err instanceof Error ? err.message : t('calendar.subscription.refresh_error', 'Failed to refresh subscription'); set({ subscriptions: get().subscriptions.map((s) =>