core: add handling for failed sync items

packages/core/src/api/index.ts

@@ -85,6 +85,7 @@ import { LazyPromise } from "../utils/lazy-promise.js";
 import { InboxApiKeys } from "./inbox-api-keys.js";
 import { Circle } from "./circle.js";
 import { Wrapped } from "./wrapped.js";
+import { FailedSyncItems } from "../collections/failed-sync-items.js";
 
 type EventSourceConstructor = new (
   uri: string,
@@ -230,6 +231,7 @@ class Database {
 
   inboxApiKeys = new InboxApiKeys(this, this.tokenManager);
   inboxItemsHistory = new InboxItemsHistory(this);
+  failedSyncItems = new FailedSyncItems(this);
 
   wrapped = new Wrapped(this);
 
@@ -353,6 +355,7 @@ class Database {
     await this.monographsCollection.init();
 
     await this.inboxItemsHistory.init();
+    await this.failedSyncItems.init();
 
     await this.trash.init();
 

packages/core/src/api/sync/__tests__/process-chunk.test.js

@@ -0,0 +1,289 @@
+/*
+This file is part of the Notesnook project (https://notesnook.com/)
+
+Copyright (C) 2023 Streetwriters (Private) Limited
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+import {
+  databaseTest,
+  loginFakeUser,
+  TEST_NOTE
+} from "../../../../__tests__/utils/index.ts";
+import { KEY_VERSION } from "../types.ts";
+import { test, expect, vi } from "vitest";
+
+const CURRENT_DATABASE_VERSION = 6.1;
+
+function chunkItem(id, keyVersion) {
+  return {
+    id,
+    v: CURRENT_DATABASE_VERSION,
+    keyVersion,
+    format: "base64",
+    alg: "xchacha20-poly1305",
+    cipher: "ciphertext",
+    iv: "iv",
+    salt: "salt",
+    length: 1
+  };
+}
+
+function decryptFailure() {
+  return Promise.reject(
+    new Error("ciphertext cannot be decrypted using that key")
+  );
+}
+
+function failedItemPayload(itemId, itemType) {
+  return {
+    itemId,
+    itemType,
+    cipher: chunkItem(itemId, KEY_VERSION.LEGACY),
+    errors: [`Failed to decrypt item ${itemId} with key version 0.`],
+    dateSynced: Date.now()
+  };
+}
+
+test("items with a key version that has no matching key are stored in failedSyncItems", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const item = chunkItem("setting-1", KEY_VERSION.DEK);
+    const keys = [{ version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } }];
+
+    // decryptMulti is never called for the DEK version group (no matching key),
+    // so only the per-item retry path runs. Make all retries fail.
+    vi.spyOn(db.storage(), "decrypt").mockImplementation(decryptFailure);
+
+    await db.syncer.sync.processChunk(
+      { type: "settingitem", count: 1, items: [item] },
+      keys,
+      { type: "fetch" }
+    );
+
+    const failedItems = await db.failedSyncItems.all.items();
+    expect(failedItems).toHaveLength(1);
+    expect(failedItems[0].itemId).toBe(item.id);
+    expect(failedItems[0].itemType).toBe("settingitem");
+    expect(failedItems[0].cipher.id).toBe(item.id);
+    expect(failedItems[0].errors.length).toBe(keys.length);
+    expect(failedItems[0].errors[0]).toContain(
+      "ciphertext cannot be decrypted"
+    );
+  }));
+
+test("items that fail to decrypt with all available keys are stored in failedSyncItems", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const item = chunkItem("setting-1", KEY_VERSION.LEGACY);
+    const keys = [
+      { version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } },
+      { version: KEY_VERSION.DEK, key: { key: "dek-key" } }
+    ];
+
+    vi.spyOn(db.storage(), "decryptMulti").mockImplementation(decryptFailure);
+    vi.spyOn(db.storage(), "decrypt").mockImplementation(decryptFailure);
+
+    await db.syncer.sync.processChunk(
+      { type: "settingitem", count: 1, items: [item] },
+      keys,
+      { type: "fetch" }
+    );
+
+    const failedItems = await db.failedSyncItems.all.items();
+    expect(failedItems).toHaveLength(1);
+    expect(failedItems[0].errors.length).toBe(keys.length);
+    expect(failedItems[0].errors[0]).toContain("key version 0");
+    expect(failedItems[0].errors[1]).toContain("key version 1");
+  }));
+
+test("items that fail batch decryption but succeed on a fallback key are decrypted and not stored as failed", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const item = chunkItem("setting-1", KEY_VERSION.DEK);
+    const keys = [
+      { version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } },
+      { version: KEY_VERSION.DEK, key: { key: "dek-key" } }
+    ];
+
+    // Batch decryption with the DEK key fails...
+    vi.spyOn(db.storage(), "decryptMulti").mockImplementation(decryptFailure);
+    // ...but the per-item retry succeeds with the legacy key.
+    const decryptedItem = JSON.stringify({
+      id: item.id,
+      type: "settingitem",
+      deleted: true,
+      dateCreated: Date.now(),
+      dateModified: Date.now()
+    });
+    vi.spyOn(db.storage(), "decrypt").mockImplementation((key) =>
+      key === keys[0].key ? Promise.resolve(decryptedItem) : decryptFailure()
+    );
+
+    await db.syncer.sync.processChunk(
+      { type: "settingitem", count: 1, items: [item] },
+      keys,
+      { type: "fetch" }
+    );
+
+    const failedItems = await db.failedSyncItems.all.items();
+    expect(failedItems).toHaveLength(0);
+    expect(db.storage().decrypt).toHaveBeenCalledTimes(1);
+  }));
+
+test("successfully decrypted items are not stored in failedSyncItems", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const item = chunkItem("setting-1", KEY_VERSION.LEGACY);
+    const keys = [{ version: KEY_VERSION.LEGACY, key: { key: "legacy-key" } }];
+
+    const decryptedItem = JSON.stringify({
+      id: item.id,
+      type: "settingitem",
+      deleted: true,
+      dateCreated: Date.now(),
+      dateModified: Date.now()
+    });
+    vi.spyOn(db.storage(), "decryptMulti").mockResolvedValue([decryptedItem]);
+
+    await db.syncer.sync.processChunk(
+      { type: "settingitem", count: 1, items: [item] },
+      keys,
+      { type: "fetch" }
+    );
+
+    const failedItems = await db.failedSyncItems.all.items();
+    expect(failedItems).toHaveLength(0);
+  }));
+
+test("failedSyncItems.add stores an item and assigns it a new id", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const itemId = await db.notes.add(TEST_NOTE);
+    const id = await db.failedSyncItems.add(failedItemPayload(itemId, "note"));
+
+    expect(id).toBeDefined();
+    expect(id).not.toBe(itemId);
+
+    const items = await db.failedSyncItems.all.items();
+    expect(items).toHaveLength(1);
+    expect(items[0].itemId).toBe(itemId);
+    expect(items[0].itemType).toBe("note");
+    expect(items[0].cipher.id).toBe(itemId);
+    expect(items[0].errors).toEqual([
+      `Failed to decrypt item ${itemId} with key version 0.`
+    ]);
+  }));
+
+test("failedSyncItems.delete removes the failed item and soft-deletes the underlying item", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const noteId = await db.notes.add(TEST_NOTE);
+    const failedId = await db.failedSyncItems.add(
+      failedItemPayload(noteId, "note")
+    );
+
+    await db.failedSyncItems.delete([failedId]);
+
+    const failedItems = await db.failedSyncItems.all.items();
+    expect(failedItems).toHaveLength(0);
+    expect(await isItemDeleted(db, "notes", noteId)).toBe(true);
+  }));
+
+test("failedSyncItems.delete groups items by type and soft-deletes in the right collections", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const noteId = await db.notes.add(TEST_NOTE);
+    const tagId = await db.tags.add({ title: "test-tag" });
+    const failedNoteId = await db.failedSyncItems.add(
+      failedItemPayload(noteId, "note")
+    );
+    const failedTagId = await db.failedSyncItems.add(
+      failedItemPayload(tagId, "tag")
+    );
+
+    await db.failedSyncItems.delete([failedNoteId, failedTagId]);
+
+    expect(await db.failedSyncItems.all.items()).toHaveLength(0);
+    expect(await isItemDeleted(db, "notes", noteId)).toBe(true);
+    expect(await isItemDeleted(db, "tags", tagId)).toBe(true);
+  }));
+
+test("failedSyncItems.delete only deletes the requested ids", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const noteId1 = await db.notes.add(TEST_NOTE);
+    const noteId2 = await db.notes.add(TEST_NOTE);
+    const failedId1 = await db.failedSyncItems.add(
+      failedItemPayload(noteId1, "note")
+    );
+    await db.failedSyncItems.add(failedItemPayload(noteId2, "note"));
+
+    await db.failedSyncItems.delete([failedId1]);
+
+    const failedItems = await db.failedSyncItems.all.items();
+    expect(failedItems).toHaveLength(1);
+    expect(failedItems[0].itemId).toBe(noteId2);
+    expect(await isItemDeleted(db, "notes", noteId1)).toBe(true);
+    expect(await db.notes.collection.get(noteId2)).toBeDefined();
+  }));
+
+test("failedSyncItems.delete skips unknown collection types gracefully", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const failedId = await db.failedSyncItems.add(
+      failedItemPayload("unknown-id", "unknown-type")
+    );
+
+    // Unknown type has no entry in SYNC_COLLECTIONS_MAP so it's skipped
+    // gracefully; the failed item row itself is still removed.
+    await expect(db.failedSyncItems.delete([failedId])).resolves.not.toThrow();
+    expect(await db.failedSyncItems.all.items()).toHaveLength(0);
+  }));
+
+test("failedSyncItems.clear removes all failed items", () =>
+  databaseTest().then(async (db) => {
+    await loginFakeUser(db);
+
+    const noteId1 = await db.notes.add(TEST_NOTE);
+    const noteId2 = await db.notes.add(TEST_NOTE);
+    await db.failedSyncItems.add(failedItemPayload(noteId1, "note"));
+    await db.failedSyncItems.add(failedItemPayload(noteId2, "note"));
+
+    await db.failedSyncItems.clear();
+
+    expect(await db.failedSyncItems.all.items()).toHaveLength(0);
+    expect(await isItemDeleted(db, "notes", noteId1)).toBeTruthy();
+    expect(await isItemDeleted(db, "notes", noteId2)).toBeTruthy();
+  }));
+
+async function isItemDeleted(db, collectionName, itemId) {
+  const item = await db
+    .sql()
+    .selectFrom(collectionName)
+    .selectAll()
+    .where("id", "==", itemId)
+    .executeTakeFirst();
+  return item?.deleted === true;
+}

packages/core/src/api/sync/index.ts

@@ -51,6 +51,7 @@ import {
   SYNC_COLLECTIONS_MAP,
   SyncableItemType,
   SyncInboxItem,
+  SyncItem,
   SyncTransferItem
 } from "./types.js";
 import { DownloadableFile } from "../../database/fs.js";
@@ -373,7 +374,7 @@ export class Sync {
     const versionMap = new Map<string, number>();
 
     for (const item of chunk.items) {
-      const keyVersion = item.keyVersion ?? KEY_VERSION.UNKNOWN;
+      const keyVersion = item.keyVersion ?? KEY_VERSION.LEGACY;
       const group = itemsByKeyVersion.get(keyVersion);
       if (group) {
         group.push(item);
@@ -383,48 +384,76 @@ export class Sync {
       versionMap.set(item.id, item.v);
     }
 
-    const unknownKeyVersionItems = itemsByKeyVersion.get(KEY_VERSION.UNKNOWN);
-    if (
-      unknownKeyVersionItems &&
-      unknownKeyVersionItems.length > 0 &&
-      keys.length > 1
-    ) {
-      this.logger.info(
-        `Decrypting ${unknownKeyVersionItems.length} items with unknown key version using all available keys.`
-      );
-      for (const item of unknownKeyVersionItems ?? []) {
-        for (const keyInfo of keys) {
-          try {
-            this.logger.info("Decrypting unknown key version item using key", {
-              keyInfo: keyInfo.version
-            });
-            const decryptedItem = await this.db
-              .storage()
-              .decrypt(keyInfo.key, item);
-            decrypted.push(decryptedItem);
-            break;
-          } catch (error) {
-            this.logger.error(
-              new Error(
-                `Failed to decrypt item ${item.id} with key version ${keyInfo.version}.`
-              )
-            );
-          }
-        }
+    const failedItems: SyncItem[] = [];
+    for (const [keyVersion, items] of itemsByKeyVersion.entries()) {
+      const keyInfo = keys.find((k) => k.version === keyVersion);
+      if (!keyInfo) {
+        this.logger.error(
+          new Error(
+            `No key found for key version ${keyVersion}. Retrying items will all available keys.`
+          )
+        );
+        failedItems.push(...items);
+        continue;
       }
-    }
-
-    for (const keyInfo of keys) {
-      const itemsToDecrypt = itemsByKeyVersion.get(keyInfo.version);
-      if (!itemsToDecrypt || itemsToDecrypt.length === 0) continue;
 
       this.logger.info("Decrypting using key", {
         keyInfo: keyInfo.version,
-        items: itemsToDecrypt.length
+        items: items.length
       });
-      decrypted.push(
-        ...(await this.db.storage().decryptMulti(keyInfo.key, itemsToDecrypt))
-      );
+      try {
+        const decryptedItems = await this.db
+          .storage()
+          .decryptMulti(keyInfo.key, items);
+        decrypted.push(...decryptedItems);
+      } catch (error) {
+        this.logger.error(
+          error,
+          `Failed to decrypt items with key version ${keyInfo.version}.`
+        );
+        failedItems.push(...items);
+      }
+    }
+
+    if (failedItems.length > 0) {
+      for (const item of failedItems) {
+        const decryptionErrors: string[] = [];
+        for (const keyInfo of keys) {
+          this.logger.info("Decrypting failed sync item using key", {
+            keyInfo: keyInfo.version
+          });
+          const decryptedItem = await this.db
+            .storage()
+            .decrypt(keyInfo.key, item)
+            .catch((error) => {
+              decryptionErrors.push(
+                `Failed to decrypt item ${item.id} with key version ${
+                  keyInfo.version
+                }. Error: ${
+                  error instanceof Error ? error.message : JSON.stringify(error)
+                }`
+              );
+              this.logger.error(
+                error,
+                `Failed to decrypt item ${item.id} with key version ${keyInfo.version}.`
+              );
+              return false;
+            });
+          if (typeof decryptedItem === "string") {
+            decrypted.push(decryptedItem);
+            break;
+          }
+        }
+        if (decryptionErrors.length === keys.length) {
+          await this.db.failedSyncItems.add({
+            itemId: item.id,
+            itemType: itemType,
+            cipher: item,
+            errors: decryptionErrors,
+            dateSynced: Date.now()
+          });
+        }
+      }
     }
 
     const deserialized: MaybeDeletedItem<Item>[] = [];

packages/core/src/api/sync/types.ts

@@ -21,8 +21,7 @@ import { Cipher } from "@notesnook/crypto";
 
 export const KEY_VERSION = {
   LEGACY: 0,
-  DEK: 1,
-  UNKNOWN: -1
+  DEK: 1
 } as const;
 
 export type KeyVersion = (typeof KEY_VERSION)[keyof typeof KEY_VERSION];

packages/core/src/collections/failed-sync-items.ts

@@ -0,0 +1,94 @@
+/*
+This file is part of the Notesnook project (https://notesnook.com/)
+
+Copyright (C) 2023 Streetwriters (Private) Limited
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+import { BaseItem, FailedSyncItem } from "../types.js";
+import Database from "../api/index.js";
+import { ICollection } from "./collection.js";
+import { SQLCollection } from "../database/sql-collection.js";
+import { isFalse } from "../database/index.js";
+import { getId } from "../utils/id.js";
+import { SYNC_COLLECTIONS_MAP } from "../api/sync/types.js";
+import type { SyncableItemType } from "../api/sync/types.js";
+import { logger } from "../logger.js";
+
+export class FailedSyncItems implements ICollection {
+  name = "failedsyncitems";
+  readonly collection: SQLCollection<"failedsyncitems", FailedSyncItem>;
+  constructor(private readonly db: Database) {
+    this.collection = new SQLCollection(
+      db.sql,
+      db.transaction,
+      "failedsyncitems",
+      db.eventManager,
+      db.sanitizer
+    );
+  }
+
+  init() {
+    return this.collection.init();
+  }
+
+  async add(item: Omit<FailedSyncItem, keyof BaseItem<"failedsyncitem">>) {
+    const now = Date.now();
+    const id = getId();
+    await this.collection.upsert({
+      type: "failedsyncitem",
+      dateCreated: now,
+      dateModified: now,
+      ...item,
+      id
+    });
+    return id;
+  }
+
+  get all() {
+    return this.collection.createFilter<FailedSyncItem>(
+      (qb) => qb.where(isFalse("deleted")),
+      this.db.options?.batchSize
+    );
+  }
+
+  async delete(ids: string[]) {
+    const groupedItems: Partial<Record<SyncableItemType, FailedSyncItem[]>> =
+      {};
+    for (const item of await this.all.items(ids)) {
+      const grouped = groupedItems[item.itemType] ?? [];
+      grouped.push(item);
+      groupedItems[item.itemType] = grouped;
+    }
+    for (const [itemType, items] of Object.entries(groupedItems)) {
+      const collectionType = SYNC_COLLECTIONS_MAP[itemType as SyncableItemType];
+      const collection = this.db[collectionType];
+      if (!collectionType || !collection) {
+        logger.error(
+          new Error(`Unknown collection type: ${itemType}`),
+          `Failed to delete failed sync items because the collection type is unknown.`
+        );
+        continue;
+      }
+      await collection.collection.softDelete(items.map((item) => item.itemId));
+    }
+    await this.collection.delete(ids);
+  }
+
+  async clear() {
+    const ids = await this.all.ids();
+    await this.delete(ids);
+  }
+}

packages/core/src/database/index.ts

@@ -41,6 +41,7 @@ import {
   Attachment,
   Color,
   ContentItem,
+  FailedSyncItem,
   HistorySession,
   InboxItemHistory,
   ItemReference,
@@ -94,6 +95,7 @@ export interface DatabaseSchema {
   vaults: SQLiteItem<Vault>;
   monographs: SQLiteItem<Monograph>;
   inboxitemshistory: SQLiteItem<InboxItemHistory>;
+  failedsyncitems: SQLiteItem<FailedSyncItem>;
 }
 
 export type RawDatabaseSchema = DatabaseSchema & {
@@ -279,6 +281,10 @@ const DataMappers: Partial<Record<ItemType, (row: any) => void>> = {
   },
   trash: (row) => {
     if (row.expiryDate) row.expiryDate = JSON.parse(row.expiryDate);
+  },
+  failedsyncitem: (row) => {
+    if (row.cipher) row.cipher = JSON.parse(row.cipher);
+    if (row.errors) row.errors = JSON.parse(row.errors);
   }
 };
 

packages/core/src/database/migrations.ts

@@ -496,6 +496,21 @@ export class NNMigrationProvider implements MigrationProvider {
             .addColumn("errorContext", "text")
             .execute();
         }
+      },
+      "a-2026-08-10": {
+        async up(db) {
+          await db.schema
+            .createTable("failedsyncitems")
+            .ifNotExists()
+            .modifyEnd(sql`without rowid`)
+            .$call(addBaseColumns)
+            .addColumn("dateSynced", "integer")
+            .addColumn("cipher", "text")
+            .addColumn("errors", "text")
+            .addColumn("itemType", "text")
+            .addColumn("itemId", "text")
+            .execute();
+        }
       }
     };
   }

packages/core/src/database/sql-collection.ts

@@ -703,7 +703,8 @@ const VALID_SORT_OPTIONS: Record<
   shortcuts: [],
   vaults: [],
   monographs: [],
-  inboxitemshistory: []
+  inboxitemshistory: [],
+  failedsyncitems: []
 };
 
 function sanitizeSortOptions(type: keyof DatabaseSchema, options: SortOptions) {

packages/core/src/types.ts

@@ -19,6 +19,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import { Cipher } from "@notesnook/crypto";
 import { isCipher } from "./utils/index.js";
+import type { SyncableItemType, SyncItem } from "./api/sync/types.js";
 
 export type TimeFormat = "12-hour" | "24-hour";
 export type DayFormat = "short" | "long";
@@ -85,6 +86,7 @@ export type Collections = {
   vaults: "vault";
   monographs: "monograph";
   inboxitemshistory: "inboxitemhistory";
+  failedsyncitems: "failedsyncitem";
 
   /**
    * @deprecated only kept here for migration purposes
@@ -117,6 +119,7 @@ export type GroupableItem = ValueOf<
     | "vault"
     | "monograph"
     | "inboxitemhistory"
+    | "failedsyncitem"
   >
 >;
 
@@ -140,6 +143,7 @@ export type ItemMap = {
   searchResult: HighlightedResult;
   monograph: Monograph;
   inboxitemhistory: InboxItemHistory;
+  failedsyncitem: FailedSyncItem;
 
   /**
    * @deprecated only kept here for migration purposes
@@ -547,6 +551,14 @@ export interface InboxItemHistory extends BaseItem<"inboxitemhistory"> {
   errorContext?: string;
 }
 
+export interface FailedSyncItem extends BaseItem<"failedsyncitem"> {
+  itemType: SyncableItemType;
+  itemId: string;
+  dateSynced: number;
+  cipher: SyncItem;
+  errors: string[];
+}
+
 export type Match = {
   prefix: string;
   match: string;