More error resilient handling of .metadata.yaml writes in local storage

Writes updated metadata to a backup file first and then copies it atomically (at least under anything that is not windows where the operation is a bit more complicated and hence not atomic)

src/octoprint/filemanager/storage.py

@@ -9,9 +9,12 @@ __copyright__ = "Copyright (C) 2014 The OctoPrint Project - Released under terms
 import logging
 import os
 import pylru
+import tempfile
 
 import octoprint.filemanager
 
+from octoprint.util import safeRename
+
 
 class StorageInterface(object):
 
@@ -867,12 +870,16 @@ class LocalFileStorage(StorageInterface):
 	def _save_metadata(self, path, metadata):
 		metadata_path = os.path.join(path, ".metadata.yaml")
 
+		fh, metadata_temporary_path = tempfile.mkstemp()
+		os.close(fh)
+
 		with self._metadata_lock:
-			with open(metadata_path, "w") as f:
+			try:
-				try:
+				with open(metadata_temporary_path, "w") as f:
 					import yaml
 					yaml.safe_dump(metadata, stream=f, default_flow_style=False, indent="  ", allow_unicode=True)
-				except:
+				safeRename(metadata_temporary_path, metadata_path, throw_error=True)
-					self._logger.exception("Error while writing .metadata.yaml to {path}".format(**locals()))
+			except:
-				else:
+				self._logger.exception("Error while writing .metadata.yaml to {path}".format(**locals()))
-					self._metadata_cache[path] = metadata
+			else:
+				self._metadata_cache[path] = metadata

src/octoprint/util/__init__.py

@@ -147,7 +147,7 @@ def findCollisionfreeName(input, extension, existingFilenames):
 	raise ValueError("Can't create a collision free filename")
 
 
-def safeRename(old, new):
+def safeRename(old, new, throw_error=False):
 	"""
 	Safely renames a file.
 
@@ -172,12 +172,14 @@ def safeRename(old, new):
 				os.rename(new, backup)
 			os.rename(old, new)
 			os.remove(backup)
-		except OSError:
+		except OSError as e:
 			# if anything went wrong, try to rename the backup file to its original name
 			logger.error("Could not perform safe rename, trying to revert")
 			if os.path.exists(backup):
-				os.remove(new)
+				silentRemove(new)
-			os.rename(backup, new)
+				os.rename(backup, new)
+			if throw_error:
+				raise e
 	else:
 		# on anything else than windows it's ooooh so much easier...
 		os.rename(old, new)