fix: keep a subscription's events when its feed arrives cut off or drops most of them at once

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",

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 }]);
+  });
 });

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<string, boolean>;
@@ -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<Partial
   if (!/BEGIN:VCALENDAR/i.test(text)) {
     throw new Error(t('calendar.subscription.not_a_feed', 'That URL did not return an iCalendar feed'));
   }
+  // A body cut off on the way (a dropped connection, a proxy's limit) parses
+  // up to the cut, and every mirror event after it looked gone upstream.
+  if (!/END:VCALENDAR\s*$/i.test(text)) {
+    throw new Error(t('calendar.subscription.feed_truncated', 'The feed was cut off before its end. Nothing was changed.'));
+  }
   assertScope(scope);
   const bytes = new TextEncoder().encode(text);
   const { blobId } = await uploadBytes(bytes, 'text/calendar', scope.accountId);
@@ -247,17 +263,20 @@ async function fetchAndParseFeed(url: string, scope: SyncScope): Promise<Partial
  * another calendar. One made for this sync has nothing to drop or update.
  * Every request names the account the sync started in, and the sync stops
  * once the client serves another (SubscriptionAccountChangedError).
+ * Resolves to what the sync has to say for lastError (events it kept that
+ * the feed left out), else null.
  */
 async function syncFeedIntoCalendar(
   calendarId: string,
   url: string,
   linkedBefore: boolean,
   scope: SyncScope,
-): Promise<void> {
+): Promise<string | null> {
   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<SubscriptionsState>()(
@@ -353,7 +384,7 @@ export const useCalendarSubscriptionsStore = create<SubscriptionsState>()(
           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<SubscriptionsState>()(
             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) =>