better transaction handling with rollback on exception

routes/api/notes.js

@@ -44,8 +44,7 @@ router.put('/:noteId', async (req, res, next) => {
 
     const history = await sql.getSingleResult("select id from notes_history where note_id = ? and date_modified_from >= ?", [noteId, historyCutoff]);
 
-    await sql.beginTransaction();
-
+    await sql.doInTransaction(async () => {
         if (history) {
             await sql.execute("update notes_history set note_title = ?, note_text = ?, encryption = ?, date_modified_to = ? where id = ?", [
                 note['detail']['note_title'],
@@ -97,20 +96,18 @@ router.put('/:noteId', async (req, res, next) => {
 
         await sql.remove("links", noteId);
 
-    for (const link in note['links'])
+        for (const link in note['links']) {
             await sql.insert("links", link);
-
-    await sql.commit();
+        }
+    });
 
     res.send({});
 });
 
 router.delete('/:noteId', async (req, res, next) => {
-    await sql.beginTransaction();
-
+    await sql.doInTransaction(async () => {
         await deleteNote(req.params.noteId, req);
-
-    await sql.commit();
+    });
 
     res.send({});
 });
@@ -165,8 +162,7 @@ router.post('/:parentNoteId/children', async (req, res, next) => {
         throw new ('Unknown target: ' + note['target']);
     }
 
-    await sql.beginTransaction();
-
+    await sql.doInTransaction(async () => {
         await sql.addAudit(audit_category.CREATE_NOTE, req, noteId);
 
         const now = utils.nowTimestamp();
@@ -189,8 +185,7 @@ router.post('/:parentNoteId/children', async (req, res, next) => {
             'date_modified': utils.nowTimestamp(),
             'is_deleted': 0
         });
-
-    await sql.commit();
+    });
 
     res.send({
         'note_id': noteId

routes/api/notes_move.js

@@ -22,14 +22,12 @@ router.put('/:noteId/moveTo/:parentId', auth.checkApiAuth, async (req, res, next
 
     const now = utils.nowTimestamp();
 
-    await sql.beginTransaction();
-
+    await sql.doInTransaction(async () => {
         await sql.execute("update notes_tree set note_pid = ?, note_pos = ?, date_modified = ? where note_id = ?",
             [parentId, newNotePos, now, noteId]);
 
         await sql.addAudit(audit_category.CHANGE_PARENT, req, noteId, null, parentId);
-
-    await sql.commit();
+    });
 
     res.send({});
 });
@@ -43,16 +41,14 @@ router.put('/:noteId/moveBefore/:beforeNoteId', async (req, res, next) => {
     if (beforeNote) {
         const now = utils.nowTimestamp();
 
-        await sql.beginTransaction();
-
+        await sql.doInTransaction(async () => {
             await sql.execute("update notes_tree set note_pos = note_pos + 1, date_modified = ? where note_id = ?", [now, beforeNoteId]);
 
             await sql.execute("update notes_tree set note_pid = ?, note_pos = ?, date_modified = ? where note_id = ?",
                 [beforeNote['note_pid'], beforeNote['note_pos'], now, noteId]);
 
             await sql.addAudit(audit_category.CHANGE_POSITION, req, noteId);
-
-        await sql.commit();
+        });
     }
 
     res.send({});
@@ -67,8 +63,7 @@ router.put('/:noteId/moveAfter/:afterNoteId', async (req, res, next) => {
     if (afterNote) {
         const now = utils.nowTimestamp();
 
-        await sql.beginTransaction();
-
+        await sql.doInTransaction(async () => {
             await sql.execute("update notes_tree set note_pos = note_pos + 1, date_modified = ? where note_pid = ? and note_pos > ? and is_deleted = 0",
                 [now, afterNote['note_pid'], afterNote['note_pos']]);
 
@@ -76,8 +71,7 @@ router.put('/:noteId/moveAfter/:afterNoteId', async (req, res, next) => {
                 [afterNote['note_pid'], afterNote['note_pos'] + 1, now, noteId]);
 
             await sql.addAudit(audit_category.CHANGE_POSITION, req, noteId);
-
-        await sql.commit();
+        });
     }
 
     res.send({});
@@ -88,9 +82,11 @@ router.put('/:noteId/expanded/:expanded', async (req, res, next) => {
     const expanded = req.params.expanded;
     const now = utils.nowTimestamp();
 
+    await sql.doInTransaction(async () => {
         await sql.execute("update notes_tree set is_expanded = ?, date_modified = ? where note_id = ?", [expanded, now, noteId]);
 
-    // no audit here, not really important
+        await sql.addAudit(audit_category.CHANGE_EXPANDED, req, noteId, null, expanded);
+    });
 
     res.send({});
 });

routes/api/settings.js

@@ -27,13 +27,11 @@ router.post('/', async (req, res, next) => {
     if (ALLOWED_OPTIONS.includes(body['name'])) {
         const optionName = await sql.getOption(body['name']);
 
-        await sql.beginTransaction();
-
+        await sql.doInTransaction(async () => {
             await sql.addAudit(audit_category.SETTINGS, req, null, optionName, body['value'], body['name']);
 
             await sql.setOption(body['name'], body['value']);
-
-        await sql.commit();
+        });
 
         res.send({});
     }

services/audit_category.js

@@ -4,6 +4,7 @@ module.exports = {
     UPDATE_CONTENT: 'CONTENT',
     UPDATE_TITLE: 'TITLE',
     CHANGE_POSITION: 'POSITION',
+    CHANGE_EXPANDED: 'EXPANDED',
     CREATE_NOTE: 'CREATE',
     DELETE_NOTE: 'DELETE',
     CHANGE_PARENT: 'PARENT',

services/change_password.js

@@ -55,15 +55,13 @@ async function changePassword(currentPassword, newPassword, req = null) {
 
     const newEncryptedDataKey = encrypt(decryptedDataKey);
 
-    await sql.beginTransaction();
-
+    await sql.doInTransaction(async () => {
         await sql.setOption('encrypted_data_key', newEncryptedDataKey);
 
         await sql.setOption('password_verification_hash', newPasswordVerificationKey);
 
         await sql.addAudit(audit_category.CHANGE_PASSWORD, req);
-
-    await sql.commit();
+    });
 
     return {
         'success': true,

services/migration.js

@@ -42,13 +42,11 @@ async function migrate() {
         try {
             log.info("Attempting migration to version " + mig.dbVersion + " with script: " + migrationSql);
 
-            await sql.beginTransaction();
-
+            await sql.doInTransaction(async () => {
                 await sql.executeScript(migrationSql);
 
                 await sql.setOption("db_version", mig.dbVersion);
-
-            await sql.commit();
+            });
 
             log.info("Migration to version " + mig.dbVersion + " has been successful.");
 

services/sql.js

@@ -109,6 +109,21 @@ async function deleteRecentAudits(category, req, noteId) {
             [category, browserId, noteId, deleteCutoff])
 }
 
+async function doInTransaction(func) {
+    try {
+        await beginTransaction();
+
+        await func();
+
+        await commit();
+    }
+    catch (e) {
+        log.error("Error executing transaction, executing rollback. Inner exception: " + e.stack);
+
+        await rollback();
+    }
+}
+
 module.exports = {
     insert,
     getSingleResult,
@@ -118,10 +133,8 @@ module.exports = {
     executeScript,
     getOption,
     setOption,
-    beginTransaction,
-    commit,
-    rollback,
     addAudit,
     deleteRecentAudits,
-    remove
+    remove,
+    doInTransaction
 };

services/sync.js

@@ -34,8 +34,7 @@ async function pullSync(cookieJar, syncLog) {
     }
 
     try {
-        await sql.beginTransaction();
-
+        await sql.doInTransaction(async () => {
             await putChanged(resp, syncLog);
 
             for (const noteId of resp.notes) {
@@ -63,12 +62,9 @@ async function pullSync(cookieJar, syncLog) {
             }
 
             await sql.setOption('last_synced_pull', resp.syncTimestamp);
-
-        await sql.commit();
+        });
     }
     catch (e) {
-        await sql.rollback();
-
         throw e;
     }
 }