Catch database assertion when item path cannot be calculated (#1217)

* Catch database assertion when item path cannot be calculated
abraunegg · Jan 15, 2021 · cef00e7 · cef00e7
1 parent bbba32c
commit cef00e7
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 10 deletions.
diff --git a/src/itemdb.d b/src/itemdb.d
@@ -388,6 +388,9 @@ final class ItemDatabase
 					}
 				} else {
 					// broken tree
+					log.vdebug("The following generated a broken tree query:");
+					log.vdebug("Drive ID: ", driveId);
+					log.vdebug("Item ID: ", id);
 					assert(0);
 				}
 			}

diff --git a/src/sync.d b/src/sync.d
@@ -1760,7 +1760,8 @@ final class SyncEngine
 								// What is the original local path for this ID in the database? Does it match 'syncFolderChildPath'
 								if (itemdb.idInLocalDatabase(driveId, item["id"].str)){
 									// item is in the database
-									string originalLocalPath = itemdb.computePath(driveId, item["id"].str);
+									string originalLocalPath = computeItemPath(driveId, item["id"].str);
+
 									if (canFind(originalLocalPath, syncFolderChildPath)){
 										JSONValue oneDriveMovedNotDeleted;
 										try {
@@ -1957,7 +1958,8 @@ final class SyncEngine
 						log.vdebug("skip_dir path to check (simple):  ", simplePathToCheck);
 						// complex path
 						if (itemdb.idInLocalDatabase(parentDriveId, parentItem)){
-							complexPathToCheck = itemdb.computePath(parentDriveId, parentItem) ~ "/" ~ driveItem["name"].str;
+							// build up complexPathToCheck
+							complexPathToCheck = computeItemPath(parentDriveId, parentItem) ~ "/" ~ driveItem["name"].str;
 							complexPathToCheck = buildNormalizedPath(complexPathToCheck);
 						} else {
 							log.vdebug("Parent details not in database - unable to compute complex path to check");
@@ -2016,8 +2018,8 @@ final class SyncEngine
 
 				// is the parent id in the database?
 				if (itemdb.idInLocalDatabase(item.driveId, item.parentId)){
-					// need to compute the full path for this file
-					path = itemdb.computePath(item.driveId, item.parentId) ~ "/" ~ item.name;
+					// Compute this item path & need the full path for this file
+					path = computeItemPath(item.driveId, item.parentId) ~ "/" ~ item.name;
 
 					// The path that needs to be checked needs to include the '/'
 					// This due to if the user has specified in skip_file an exclusive path: '/path/file' - that is what must be matched
@@ -2050,7 +2052,8 @@ final class SyncEngine
 			} else {
 				// Why was this unwanted?
 				if (path.empty) {
-					path = itemdb.computePath(item.driveId, item.parentId) ~ "/" ~ item.name;
+					// Compute this item path & need the full path for this file
+					path = computeItemPath(item.driveId, item.parentId) ~ "/" ~ item.name;
 				}
 				// Microsoft OneNote container objects present as neither folder or file but has file size
 				if ((!isItemFile(driveItem)) && (!isItemFolder(driveItem)) && (hasFileSize(driveItem))) {
@@ -2069,8 +2072,8 @@ final class SyncEngine
 		if (!unwanted) {
 			// Is the item parent in the local database?
 			if (itemdb.idInLocalDatabase(item.driveId, item.parentId)){
-				// compute the item path to see if the path is excluded
-				path = itemdb.computePath(item.driveId, item.parentId) ~ "/" ~ item.name;
+				// compute the item path to see if the path is excluded & need the full path for this file
+				path = computeItemPath(item.driveId, item.parentId) ~ "/" ~ item.name;
 				path = buildNormalizedPath(path);
 				if (selectiveSync.isPathExcludedViaSyncList(path)) {
 					// selective sync advised to skip, however is this a file and are we configured to upload / download files in the root?
@@ -2165,7 +2168,8 @@ final class SyncEngine
 			// Is the item in the local database
 			if (itemdb.idInLocalDatabase(item.driveId, item.id)){
 				log.vdebug("OneDrive item ID is present in local database");
-				oldPath = itemdb.computePath(item.driveId, item.id);
+				// Compute this item path
+				oldPath = computeItemPath(item.driveId, item.id);
 				// Query DB for existing local item in specified path
 				string itemSource = "database";
 				if (!isItemSynced(oldItem, oldPath, itemSource)) {
@@ -2781,8 +2785,11 @@ final class SyncEngine
 	{
 		foreach_reverse (i; idsToDelete) {
 			Item item;
+			string path;
 			if (!itemdb.selectById(i[0], i[1], item)) continue; // check if the item is in the db
-			const(string) path = itemdb.computePath(i[0], i[1]);
+			// Compute this item path
+			path = computeItemPath(i[0], i[1]);
+			// Try to delete item object
 			log.log("Trying to delete item ", path);
 			if (!dryRun) {
 				// Actually process the database entry removal
@@ -3100,7 +3107,9 @@ final class SyncEngine
 		string path;
 
 		// Compute this item path early as we we use this path often
-		path = itemdb.computePath(item.driveId, item.id);
+		path = computeItemPath(item.driveId, item.id);
+
+		// item.id was in the database associated with the item.driveId specified
 		log.vlog("Processing ", buildNormalizedPath(path));
 
 		// What type of DB item are we processing
@@ -6199,4 +6208,22 @@ final class SyncEngine
 			log.error("ERROR: onedrive.getSharedWithMe call returned an invalid JSON Object");
 		}
 	}
+
+	// Query itemdb.computePath() and catch potential assert when DB consistency issue occurs
+	string computeItemPath(string thisDriveId, string thisItemId)
+	{
+		string calculatedPath;
+		log.vdebug("Attempting to calculate local filesystem path for ", thisDriveId, " and ", thisItemId);
+		try {
+			calculatedPath = itemdb.computePath(thisDriveId, thisItemId);
+		} catch (core.exception.AssertError) {
+			// broken tree in the database, we cant compute the path for this item id, exit
+			log.error("ERROR: A database consistency issue has been caught. A --resync is needed to rebuild the database.");
+			// Must exit here to preserve data
+			exit(-1);
+		}
+
+		// return calculated path as string
+		return calculatedPath;
+	}
 }