filesystem retry fixes (#354)

* add more forgiving retries with filesystem

* fall back to FindFirstFile if access prevents us from using GetFileAttributes

* only validate hash if we have a complete payload in http client

* changelog

3 files changed, 103 insertions, 59 deletions

diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp
index 2562d8244..f7f9e3abb 100644
--- a/src/zen/cmds/builds_cmd.cpp
+++ b/src/zen/cmds/builds_cmd.cpp
@@ -182,65 +182,74 @@ namespace {
 
 	std::filesystem::path MakeSafeAbsolutePath(const std::string PathString) { return MakeSafeAbsolutePath(StringToPath(PathString)); }
 
-	void RenameFileWithRetry(const std::filesystem::path& SourcePath, const std::filesystem::path& TargetPath)
+	bool IsFileWithRetry(const std::filesystem::path& Path)
 	{
 		std::error_code Ec;
-		RenameFile(SourcePath, TargetPath, Ec);
+		bool			Result = IsFile(Path, Ec);
 		for (size_t Retries = 0; Ec && Retries < 3; Retries++)
 		{
 			Sleep(100 + int(Retries * 50));
 			Ec.clear();
-			RenameFile(SourcePath, TargetPath, Ec);
+			Result = IsFile(Path, Ec);
 		}
 		if (Ec)
 		{
-			zen::ThrowSystemError(Ec.value(), Ec.message());
+			throw std::system_error(std::error_code(Ec.value(), std::system_category()),
+									fmt::format("Check path '{}' is file failed with: {} ({})", Path, Ec.message(), Ec.value()));
 		}
+		return Result;
 	}
 
-	bool IsFileWithRetry(const std::filesystem::path& Path)
+	bool SetFileReadOnlyWithRetry(const std::filesystem::path& Path, bool ReadOnly)
 	{
 		std::error_code Ec;
-		bool			Result = IsFile(Path, Ec);
+		bool			Result = SetFileReadOnly(Path, ReadOnly, Ec);
 		for (size_t Retries = 0; Ec && Retries < 3; Retries++)
 		{
 			Sleep(100 + int(Retries * 50));
+			if (!IsFileWithRetry(Path))
+			{
+				return false;
+			}
 			Ec.clear();
-			Result = IsFile(Path, Ec);
+			Result = SetFileReadOnly(Path, ReadOnly, Ec);
 		}
 		if (Ec)
 		{
-			zen::ThrowSystemError(Ec.value(), Ec.message());
+			throw std::system_error(
+				std::error_code(Ec.value(), std::system_category()),
+				fmt::format("Failed {} read only flag for '{}' failed with: {}", ReadOnly ? "setting" : "clearing", Path, Ec.message()));
 		}
 		return Result;
 	}
 
-	bool SetFileReadOnlyWithRetry(const std::filesystem::path& Path, bool ReadOnly)
+	void RenameFileWithRetry(const std::filesystem::path& SourcePath, const std::filesystem::path& TargetPath)
 	{
 		std::error_code Ec;
-		bool			Result = SetFileReadOnly(Path, ReadOnly, Ec);
-		for (size_t Retries = 0; Ec && Retries < 3; Retries++)
+		RenameFile(SourcePath, TargetPath, Ec);
+		for (size_t Retries = 0; Ec && Retries < 10; Retries++)
 		{
-			Sleep(100 + int(Retries * 50));
-			if (!IsFileWithRetry(Path))
+			ZEN_ASSERT_SLOW(IsFile(SourcePath));
+			if (Retries > 5)
 			{
-				return false;
+				ZEN_CONSOLE("Unable to overwrite file {} ({}: {}), retrying...", TargetPath, Ec.value(), Ec.message());
 			}
+			Sleep(50 + int(Retries * 150));
 			Ec.clear();
-			Result = SetFileReadOnly(Path, ReadOnly, Ec);
+			RenameFile(SourcePath, TargetPath, Ec);
 		}
 		if (Ec)
 		{
-			zen::ThrowSystemError(Ec.value(), Ec.message());
+			throw std::system_error(std::error_code(Ec.value(), std::system_category()),
+									fmt::format("Rename from '{}' to '{}' failed with: {}", SourcePath, TargetPath, Ec.message()));
 		}
-		return Result;
 	}
 
 	void RemoveFileWithRetry(const std::filesystem::path& Path)
 	{
 		std::error_code Ec;
 		RemoveFile(Path, Ec);
-		for (size_t Retries = 0; Ec && Retries < 3; Retries++)
+		for (size_t Retries = 0; Ec && Retries < 6; Retries++)
 		{
 			Sleep(100 + int(Retries * 50));
 			if (!IsFileWithRetry(Path))
@@ -252,7 +261,8 @@ namespace {
 		}
 		if (Ec)
 		{
-			zen::ThrowSystemError(Ec.value(), Ec.message());
+			throw std::system_error(std::error_code(Ec.value(), std::system_category()),
+									fmt::format("Removing file '{}' failed with: {}", Path, Ec.message()));
 		}
 	}
 
@@ -272,7 +282,8 @@ namespace {
 		}
 		if (Ec)
 		{
-			zen::ThrowSystemError(Ec.value(), Ec.message());
+			throw std::system_error(std::error_code(Ec.value(), std::system_category()),
+									fmt::format("Removing directory '{}' failed with: {}", Path, Ec.message()));
 		}
 	}
 
diff --git a/src/zencore/filesystem.cpp b/src/zencore/filesystem.cpp
index 4ec563ba3..8ee21d9ab 100644
--- a/src/zencore/filesystem.cpp
+++ b/src/zencore/filesystem.cpp
@@ -251,6 +251,51 @@ MakeFileModeReadOnly(uint32_t FileMode, bool ReadOnly)
 	return ReadOnly ? (FileMode & ~FileModeWriteEnableFlags) : (FileMode | FileModeWriteEnableFlags);
 }
 
+#if ZEN_PLATFORM_WINDOWS
+
+static DWORD
+WinGetFileAttributes(const std::filesystem::path& Path, std::error_code& Ec)
+{
+	DWORD Attributes = ::GetFileAttributes(Path.native().c_str());
+	if (Attributes == INVALID_FILE_ATTRIBUTES)
+	{
+		DWORD LastError = GetLastError();
+		switch (LastError)
+		{
+			case ERROR_FILE_NOT_FOUND:
+			case ERROR_PATH_NOT_FOUND:
+			case ERROR_BAD_NETPATH:
+			case ERROR_INVALID_DRIVE:
+				break;
+			case ERROR_ACCESS_DENIED:
+				{
+					WIN32_FIND_DATA FindData;
+					HANDLE			FindHandle = ::FindFirstFile(Path.native().c_str(), &FindData);
+					if (FindHandle == INVALID_HANDLE_VALUE)
+					{
+						DWORD LastFindError = GetLastError();
+						if (LastFindError != ERROR_FILE_NOT_FOUND)
+						{
+							Ec = MakeErrorCode(LastError);
+						}
+					}
+					else
+					{
+						CloseHandle(FindHandle);
+						Attributes = FindData.dwFileAttributes;
+					}
+				}
+				break;
+			default:
+				Ec = MakeErrorCode(LastError);
+				break;
+		}
+	}
+	return Attributes;
+}
+
+#endif	// ZEN_PLATFORM_WINDOWS
+
 bool
 RemoveDirNative(const std::filesystem::path& Path, std::error_code& Ec)
 {
@@ -286,7 +331,12 @@ RemoveFileNative(const std::filesystem::path& Path, bool ForceRemoveReadOnlyFile
 	{
 		if (ForceRemoveReadOnlyFiles)
 		{
-			DWORD FileAttributes = ::GetFileAttributes(NativePath);
+			DWORD FileAttributes = WinGetFileAttributes(NativePath, Ec);
+			if (Ec)
+			{
+				return false;
+			}
+
 			if ((FileAttributes != INVALID_FILE_ATTRIBUTES) && IsFileAttributeReadOnly(FileAttributes) != 0)
 			{
 				::SetFileAttributes(NativePath, MakeFileAttributeReadOnly(FileAttributes, false));
@@ -1597,21 +1647,13 @@ bool
 IsFile(const std::filesystem::path& Path, std::error_code& Ec)
 {
 #if ZEN_PLATFORM_WINDOWS
-	DWORD Attributes = ::GetFileAttributes(Path.native().c_str());
+	DWORD Attributes = WinGetFileAttributes(Path, Ec);
+	if (Ec)
+	{
+		return false;
+	}
 	if (Attributes == INVALID_FILE_ATTRIBUTES)
 	{
-		DWORD LastError = GetLastError();
-		switch (LastError)
-		{
-			case ERROR_FILE_NOT_FOUND:
-			case ERROR_PATH_NOT_FOUND:
-			case ERROR_BAD_NETPATH:
-			case ERROR_INVALID_DRIVE:
-				break;
-			default:
-				Ec = MakeErrorCode(LastError);
-				break;
-		}
 		return false;
 	}
 	return (Attributes & FILE_ATTRIBUTE_DIRECTORY) == 0;
@@ -1651,21 +1693,13 @@ bool
 IsDir(const std::filesystem::path& Path, std::error_code& Ec)
 {
 #if ZEN_PLATFORM_WINDOWS
-	DWORD Attributes = ::GetFileAttributes(Path.native().c_str());
+	DWORD Attributes = WinGetFileAttributes(Path, Ec);
+	if (Ec)
+	{
+		return false;
+	}
 	if (Attributes == INVALID_FILE_ATTRIBUTES)
 	{
-		DWORD LastError = GetLastError();
-		switch (LastError)
-		{
-			case ERROR_FILE_NOT_FOUND:
-			case ERROR_PATH_NOT_FOUND:
-			case ERROR_BAD_NETPATH:
-			case ERROR_INVALID_DRIVE:
-				break;
-			default:
-				Ec = MakeErrorCode(LastError);
-				break;
-		}
 		return false;
 	}
 	return (Attributes & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY;
@@ -2492,13 +2526,7 @@ PickDefaultSystemRootDirectory()
 uint32_t
 GetFileAttributes(const std::filesystem::path& Filename, std::error_code& Ec)
 {
-	DWORD Attributes = ::GetFileAttributes(Filename.native().c_str());
-	if (Attributes == INVALID_FILE_ATTRIBUTES)
-	{
-		Ec = MakeErrorCodeFromLastError();
-		return 0;
-	}
-	return (uint32_t)Attributes;
+	return WinGetFileAttributes(Filename, Ec);
 }
 
 uint32_t
@@ -2594,6 +2622,11 @@ SetFileReadOnly(const std::filesystem::path& Filename, bool ReadOnly, std::error
 	{
 		return false;
 	}
+	if (CurrentAttributes == INVALID_FILE_ATTRIBUTES)
+	{
+		Ec = MakeErrorCode(ERROR_FILE_NOT_FOUND);
+		return false;
+	}
 	uint32_t NewAttributes = MakeFileAttributeReadOnly(CurrentAttributes, ReadOnly);
 	if (CurrentAttributes != NewAttributes)
 	{
diff --git a/src/zenhttp/httpclient.cpp b/src/zenhttp/httpclient.cpp
index f3baf37ce..763f3262a 100644
--- a/src/zenhttp/httpclient.cpp
+++ b/src/zenhttp/httpclient.cpp
@@ -410,6 +410,11 @@ ValidatePayload(cpr::Response& Response, std::unique_ptr<detail::TempPayloadFile
 		}
 	}
 
+	if (Response.status_code == (long)HttpResponseCode::PartialContent)
+	{
+		return true;
+	}
+
 	if (auto JupiterHash = Response.header.find("X-Jupiter-IoHash"); JupiterHash != Response.header.end())
 	{
 		IoHash ExpectedPayloadHash;
@@ -427,11 +432,6 @@ ValidatePayload(cpr::Response& Response, std::unique_ptr<detail::TempPayloadFile
 		}
 	}
 
-	if (Response.status_code == (long)HttpResponseCode::PartialContent)
-	{
-		return true;
-	}
-
 	if (auto ContentType = Response.header.find("Content-Type"); ContentType != Response.header.end())
 	{
 		if (ContentType->second == "application/x-ue-comp")