Fixed up PutLargeObject() error handling

2 files changed, 28 insertions, 16 deletions

diff --git a/zenserver/cache/cachestore.cpp b/zenserver/cache/cachestore.cpp
index e0dd57902..1db58c1e6 100644
--- a/zenserver/cache/cachestore.cpp
+++ b/zenserver/cache/cachestore.cpp
@@ -2,6 +2,7 @@
 
 #include "cachestore.h"
 
+#include <zencore/except.h>
 #include <zencore/windows.h>
 
 #include <fmt/core.h>
@@ -1019,9 +1020,7 @@ ZenCacheDiskLayer::CacheBucket::Get(const zen::IoHash& HashKey, ZenCacheValue& O
 	{
 		zen::RwLock::SharedLockScope _(m_IndexLock);
 
-		auto it = m_Index.find(HashKey);
-
-		if (it != m_Index.end())
+		if (auto it = m_Index.find(HashKey); it != m_Index.end())
 		{
 			OutValue.Value = IoBufferBuilder::MakeFromFileHandle(m_SobsFile.Handle(), it->second.Offset, it->second.Size);
 
@@ -1050,10 +1049,14 @@ void
 ZenCacheDiskLayer::CacheBucket::Put(const zen::IoHash& HashKey, const ZenCacheValue& Value)
 {
 	if (!m_Ok)
+	{
 		return;
+	}
 
 	if (Value.Value.Size() >= m_LargeObjectThreshold)
-		PutLargeObject(HashKey, Value);
+	{
+		return PutLargeObject(HashKey, Value);
+	}
 
 	// Small object put
 
@@ -1096,24 +1099,33 @@ ZenCacheDiskLayer::CacheBucket::Flush()
 void
 ZenCacheDiskLayer::CacheBucket::PutLargeObject(const zen::IoHash& HashKey, const ZenCacheValue& Value)
 {
-	zen::WideStringBuilder<128> dataFilePath;
+	zen::WideStringBuilder<128> DataFilePath;
+	BuildPath(DataFilePath, HashKey);
 
-	BuildPath(dataFilePath, HashKey);
-
-	CAtlTemporaryFile dataFile;
+	// TODO: replace this with a more efficient implementation with proper atomic rename
 
-	HRESULT hRes = dataFile.Create(m_BucketDir.c_str());
+	CAtlTemporaryFile DataFile;
 
-	hRes = dataFile.Write(Value.Value.Data(), gsl::narrow<DWORD>(Value.Value.Size()));
+	HRESULT hRes = DataFile.Create(m_BucketDir.c_str());
 
 	if (FAILED(hRes))
 	{
-		// TODO: report error! and delete temp file
+		zen::ThrowSystemException(hRes, "Failed to open temporary file for put at '{}'"_format(m_BucketDir));
+	}
 
-		return;
+	hRes = DataFile.Write(Value.Value.Data(), gsl::narrow<DWORD>(Value.Value.Size()));
+
+	if (FAILED(hRes))
+	{
+		zen::ThrowSystemException(hRes, "Failed to write payload ({} bytes) to file"_format(NiceBytes(Value.Value.Size())));
 	}
 
-	hRes = dataFile.Close(dataFilePath.c_str());
+	hRes = DataFile.Close(DataFilePath.c_str());
+
+	if (FAILED(hRes))
+	{
+		zen::ThrowSystemException(hRes, "Failed to finalize file '{}'"_format(zen::WideToUtf8(DataFilePath)));
+	}
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -1238,7 +1250,7 @@ ZenCacheTracker::TrackAccess(std::string_view Bucket, const zen::IoHash& HashKey
 	ZEN_UNUSED(HashKey);
 }
 
-void 
+void
 ZenCacheTracker::Flush()
 {
 }
diff --git a/zenserver/cache/cachestore.h b/zenserver/cache/cachestore.h
index 41e47d87d..3e5fa5fc8 100644
--- a/zenserver/cache/cachestore.h
+++ b/zenserver/cache/cachestore.h
@@ -158,8 +158,8 @@ public:
 	ZenCacheStore(zen::CasStore& Cas, const std::filesystem::path& RootDir);
 	~ZenCacheStore();
 
-	virtual bool Get(std::string_view Bucket, const zen::IoHash& HashKey, ZenCacheValue& OutValue);
-	virtual void Put(std::string_view Bucket, const zen::IoHash& HashKey, const ZenCacheValue& Value);
+	bool Get(std::string_view Bucket, const zen::IoHash& HashKey, ZenCacheValue& OutValue);
+	void Put(std::string_view Bucket, const zen::IoHash& HashKey, const ZenCacheValue& Value);
 
 private:
 	std::filesystem::path m_RootDir;