From aff9ce97eee43e76b4fb6d61cb4a3e5f744e1b28 Mon Sep 17 00:00:00 2001
From: zadam <zadam.apps@gmail.com>
Date: Mon, 16 Dec 2019 22:00:44 +0100
Subject: [PATCH] small sync fixes

---
 src/public/javascripts/entities/branch.js     |  1 -
 src/public/javascripts/services/tree_cache.js |  2 +-
 src/public/javascripts/services/ws.js         | 30 ++++++++++++-------
 src/services/notes.js                         |  8 +++--
 4 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/src/public/javascripts/entities/branch.js b/src/public/javascripts/entities/branch.js
index 396e1c229..b06a9e523 100644
--- a/src/public/javascripts/entities/branch.js
+++ b/src/public/javascripts/entities/branch.js
@@ -6,7 +6,6 @@ class Branch {
         this.branchId = row.branchId;
         /** @param {string} */
         this.noteId = row.noteId;
-        this.note = null;
         /** @param {string} */
         this.parentNoteId = row.parentNoteId;
         /** @param {int} */
diff --git a/src/public/javascripts/services/tree_cache.js b/src/public/javascripts/services/tree_cache.js
index ff8a910c2..9d6257cf8 100644
--- a/src/public/javascripts/services/tree_cache.js
+++ b/src/public/javascripts/services/tree_cache.js
@@ -133,7 +133,7 @@ class TreeCache {
 
     /** @return {Promise<NoteShort[]>} */
     async getNotes(noteIds, silentNotFoundError = false) {
-        const missingNoteIds = noteIds.filter(noteId => this.notes[noteId] === undefined);
+        const missingNoteIds = noteIds.filter(noteId => !this.notes[noteId]);
 
         if (missingNoteIds.length > 0) {
             await this.reloadNotes(missingNoteIds);
diff --git a/src/public/javascripts/services/ws.js b/src/public/javascripts/services/ws.js
index 463c1813c..b6bb40882 100644
--- a/src/public/javascripts/services/ws.js
+++ b/src/public/javascripts/services/ws.js
@@ -64,13 +64,16 @@ async function handleMessage(event) {
                 await consumeQueuePromise;
             }
 
-            // it's my turn so start it up
-            consumeQueuePromise = consumeSyncData();
+            try {
+                // it's my turn so start it up
+                consumeQueuePromise = consumeSyncData();
 
-            await consumeQueuePromise;
-
-            // finish and set to null to signal somebody else can pick it up
-            consumeQueuePromise = null;
+                await consumeQueuePromise;
+            }
+            finally {
+                // finish and set to null to signal somebody else can pick it up
+                consumeQueuePromise = null;
+            }
         }
     }
     else if (message.type === 'sync-hash-check-failed') {
@@ -113,6 +116,15 @@ function checkSyncIdListeners() {
         .forEach(l => console.log(`Waiting for syncId ${l.desiredSyncId} while current is ${lastProcessedSyncId} for ${Math.floor((Date.now() - l.start) / 1000)}s`));
 }
 
+async function runSafely(syncHandler, syncData) {
+    try {
+        return await syncHandler(syncData);
+    }
+    catch (e) {
+        console.log(`Sync handler failed with ${e.message}: ${e.stack}`);
+    }
+}
+
 async function consumeSyncData() {
     if (syncDataQueue.length > 0) {
         const allSyncData = syncDataQueue;
@@ -126,8 +138,8 @@ async function consumeSyncData() {
 
         // the update process should be synchronous as a whole but individual handlers can run in parallel
         await Promise.all([
-            ...allSyncMessageHandlers.map(syncHandler => syncHandler(allSyncData)),
-            ...outsideSyncMessageHandlers.map(syncHandler => syncHandler(outsideSyncData))
+            ...allSyncMessageHandlers.map(syncHandler => runSafely(syncHandler, allSyncData)),
+            ...outsideSyncMessageHandlers.map(syncHandler => runSafely(syncHandler, outsideSyncData))
         ]);
 
         lastProcessedSyncId = Math.max(lastProcessedSyncId, allSyncData[allSyncData.length - 1].id);
@@ -171,8 +183,6 @@ async function sendPing() {
 setTimeout(() => {
     ws = connectWebSocket();
 
-    lastAcceptedSyncId = glob.maxSyncIdAtLoad;
-    lastProcessedSyncId = glob.maxSyncIdAtLoad;
     lastPingTs = Date.now();
 
     setInterval(sendPing, 1000);
diff --git a/src/services/notes.js b/src/services/notes.js
index 26a743b3f..d6a7052cf 100644
--- a/src/services/notes.js
+++ b/src/services/notes.js
@@ -3,7 +3,6 @@ const sqlInit = require('./sql_init');
 const optionService = require('./options');
 const dateUtils = require('./date_utils');
 const syncTableService = require('./sync_table');
-const attributeService = require('./attributes');
 const eventService = require('./events');
 const repository = require('./repository');
 const cls = require('../services/cls');
@@ -462,8 +461,11 @@ async function eraseDeletedNotes() {
         return;
     }
 
-    // it's better to not use repository for this because it will complain about saving protected notes
-    // out of protected session, also we don't want these changes to be synced (since they are done on all instances anyway)
+    // it's better to not use repository for this because:
+    // - it would complain about saving protected notes out of protected session
+    // - we don't want these changes to be synced (since they are done on all instances anyway)
+    // - we don't want change the hash since this erasing happens on each instance separately
+    //   and changing the hash would fire up the sync errors temporarily
 
     // setting contentLength to zero would serve no benefit and it leaves potentially useful trail
     await sql.executeMany(`