error handling fixes and typo fixes

2 files changed, 60 insertions, 54 deletions

diff --git a/src/zenremotestore/projectstore/remoteprojectstore.cpp b/src/zenremotestore/projectstore/remoteprojectstore.cpp
index ed803e708..d26738dde 100644
--- a/src/zenremotestore/projectstore/remoteprojectstore.cpp
+++ b/src/zenremotestore/projectstore/remoteprojectstore.cpp
@@ -442,9 +442,8 @@ namespace remotestore_impl {
 			bool CouldCompress = CompressedBuffer::CompressToStream(
 				CompositeBuffer(SharedBuffer(RawData)),
 				[&](uint64_t SourceOffset, uint64_t SourceSize, uint64_t Offset, const CompositeBuffer& RangeBuffer) {
-					ZEN_UNUSED(SourceOffset);
-					CompressedFile.Write(RangeBuffer, Offset);
 					ZEN_UNUSED(SourceOffset, SourceSize);
+					CompressedFile.Write(RangeBuffer, Offset);
 				},
 				Compressor,
 				CompressionLevel);
@@ -1235,8 +1234,8 @@ namespace remotestore_impl {
 						ReportMessage(Context.OptionalJobContext,
 									  fmt::format("Failed to load attachments with {} chunks ({}): {}",
 												  Chunks.size(),
-												  RemoteResult.GetError(),
-												  RemoteResult.GetErrorReason()));
+												  Result.ErrorCode,
+												  Result.Reason));
 						Info.MissingAttachmentCount.fetch_add(1);
 						if (!Context.IgnoreMissingAttachments)
 						{
@@ -1541,7 +1540,7 @@ namespace remotestore_impl {
 							catch (const std::exception& Ex)
 							{
 								RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::InternalServerError),
-													  fmt::format("Failed save block attachment {}", BlockHash),
+													  fmt::format("Failed to save block attachment {}", BlockHash),
 													  Ex.what());
 							}
 						},
@@ -2369,7 +2368,7 @@ namespace remotestore_impl {
 					catch (const std::exception& Ex)
 					{
 						RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::InternalServerError),
-											  fmt::format("To upload attachment {}", RawHash),
+											  fmt::format("Failed to upload attachment {}", RawHash),
 											  Ex.what());
 					}
 				},
@@ -2471,7 +2470,7 @@ namespace remotestore_impl {
 						catch (const std::exception& Ex)
 						{
 							RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::InternalServerError),
-												  fmt::format("Failed to buck upload {} attachments", NeededChunks.size()),
+												  fmt::format("Failed to bulk upload {} attachments", NeededChunks.size()),
 												  Ex.what());
 						}
 					},
@@ -2689,8 +2688,6 @@ BuildContainer(CidStore&																	ChunkStore,
 		UploadAttachments.erase(AttachmentHash);
 	}
 
-	std::unordered_set<IoHash, IoHash::Hasher> ChunkedHashes;
-
 	std::vector<remotestore_impl::ChunkedFile> ChunkedFiles = ChunkAttachments(WorkerPool, AttachmentsToChunk, OptionalContext);
 	if (remotestore_impl::IsCancelled(OptionalContext))
 	{
@@ -2849,7 +2846,7 @@ BuildContainer(CidStore&																	ChunkStore,
 		OptionalContext,
 		fmt::format("Assembling {} attachments and {} chunked parts from {} ops into blocks and loose attachments",
 					SortedUploadAttachments.size(),
-					ChunkedHashes.size(),
+					ChunkedChunkCount,
 					TotalOpCount));
 
 	if (remotestore_impl::IsCancelled(OptionalContext))
@@ -2857,7 +2854,7 @@ BuildContainer(CidStore&																	ChunkStore,
 		return {};
 	}
 
-	size_t ChunkAssembleCount = SortedUploadAttachments.size() + ChunkedHashes.size();
+	size_t ChunkAssembleCount = SortedUploadAttachments.size() + ChunkedChunkCount;
 	size_t ChunksAssembled	  = 0;
 	remotestore_impl::ReportMessage(OptionalContext,
 									fmt::format("Assembling {} attachments from {} ops into blocks", ChunkAssembleCount, TotalOpCount));
@@ -3045,9 +3042,7 @@ BuildContainer(CidStore&																	ChunkStore,
 													  return Compressed.GetCompressed();
 												  });
 
-								//								ResolveLock.WithExclusiveLock([ChunkHash, &LargeChunkHashes]() {
 								LargeChunkHashes.insert(ChunkHash);
-								//								});
 							}
 							else
 							{
@@ -3136,10 +3131,10 @@ BuildContainer(CidStore&																	ChunkStore,
 		return {};
 	}
 
-	CbObjectWriter			OplogContinerWriter;
+	CbObjectWriter			OplogContainerWriter;
 	RwLock::SharedLockScope _(BlocksLock);
-	OplogContinerWriter.AddBinary("ops"sv, CompressedOpsSection.GetCompressed().Flatten().AsIoBuffer());
-	OplogContinerWriter.BeginArray("blocks"sv);
+	OplogContainerWriter.AddBinary("ops"sv, CompressedOpsSection.GetCompressed().Flatten().AsIoBuffer());
+	OplogContainerWriter.BeginArray("blocks"sv);
 	{
 		for (const ChunkBlockDescription& B : Blocks)
 		{
@@ -3148,78 +3143,78 @@ BuildContainer(CidStore&																	ChunkStore,
 			{
 				ZEN_ASSERT(B.BlockHash != IoHash::Zero);
 
-				OplogContinerWriter.BeginObject();
+				OplogContainerWriter.BeginObject();
 				{
-					OplogContinerWriter.AddBinaryAttachment("rawhash"sv, B.BlockHash);
-					OplogContinerWriter.BeginArray("chunks"sv);
+					OplogContainerWriter.AddBinaryAttachment("rawhash"sv, B.BlockHash);
+					OplogContainerWriter.BeginArray("chunks"sv);
 					{
 						for (const IoHash& RawHash : B.ChunkRawHashes)
 						{
-							OplogContinerWriter.AddHash(RawHash);
+							OplogContainerWriter.AddHash(RawHash);
 						}
 					}
-					OplogContinerWriter.EndArray();	 // "chunks"
+					OplogContainerWriter.EndArray();  // "chunks"
 				}
-				OplogContinerWriter.EndObject();
+				OplogContainerWriter.EndObject();
 				continue;
 			}
 
 			ZEN_ASSERT(B.BlockHash == IoHash::Zero);
-			OplogContinerWriter.BeginObject();
+			OplogContainerWriter.BeginObject();
 			{
-				OplogContinerWriter.BeginArray("chunks"sv);
+				OplogContainerWriter.BeginArray("chunks"sv);
 				{
 					for (const IoHash& RawHash : B.ChunkRawHashes)
 					{
-						OplogContinerWriter.AddBinaryAttachment(RawHash);
+						OplogContainerWriter.AddBinaryAttachment(RawHash);
 					}
 				}
-				OplogContinerWriter.EndArray();
+				OplogContainerWriter.EndArray();
 			}
-			OplogContinerWriter.EndObject();
+			OplogContainerWriter.EndObject();
 		}
 	}
-	OplogContinerWriter.EndArray();	 // "blocks"sv
-	OplogContinerWriter.BeginArray("chunkedfiles"sv);
+	OplogContainerWriter.EndArray();  // "blocks"sv
+	OplogContainerWriter.BeginArray("chunkedfiles"sv);
 	{
 		for (const remotestore_impl::ChunkedFile& F : ChunkedFiles)
 		{
-			OplogContinerWriter.BeginObject();
+			OplogContainerWriter.BeginObject();
 			{
-				OplogContinerWriter.AddHash("rawhash"sv, F.Chunked.Info.RawHash);
-				OplogContinerWriter.AddInteger("rawsize"sv, F.Chunked.Info.RawSize);
-				OplogContinerWriter.BeginArray("chunks"sv);
+				OplogContainerWriter.AddHash("rawhash"sv, F.Chunked.Info.RawHash);
+				OplogContainerWriter.AddInteger("rawsize"sv, F.Chunked.Info.RawSize);
+				OplogContainerWriter.BeginArray("chunks"sv);
 				{
 					for (const IoHash& RawHash : F.Chunked.Info.ChunkHashes)
 					{
-						OplogContinerWriter.AddHash(RawHash);
+						OplogContainerWriter.AddHash(RawHash);
 					}
 				}
-				OplogContinerWriter.EndArray();	 // "chunks"
-				OplogContinerWriter.BeginArray("sequence"sv);
+				OplogContainerWriter.EndArray();  // "chunks"
+				OplogContainerWriter.BeginArray("sequence"sv);
 				{
 					for (uint32_t ChunkIndex : F.Chunked.Info.ChunkSequence)
 					{
-						OplogContinerWriter.AddInteger(ChunkIndex);
+						OplogContainerWriter.AddInteger(ChunkIndex);
 					}
 				}
-				OplogContinerWriter.EndArray();	 // "sequence"
+				OplogContainerWriter.EndArray();  // "sequence"
 			}
-			OplogContinerWriter.EndObject();
+			OplogContainerWriter.EndObject();
 		}
 	}
-	OplogContinerWriter.EndArray();	 // "chunkedfiles"sv
+	OplogContainerWriter.EndArray();  // "chunkedfiles"sv
 
-	OplogContinerWriter.BeginArray("chunks"sv);
+	OplogContainerWriter.BeginArray("chunks"sv);
 	{
 		for (const IoHash& AttachmentHash : LargeChunkHashes)
 		{
-			OplogContinerWriter.AddBinaryAttachment(AttachmentHash);
+			OplogContainerWriter.AddBinaryAttachment(AttachmentHash);
 		}
 	}
-	OplogContinerWriter.EndArray();	 // "chunks"
+	OplogContainerWriter.EndArray();  // "chunks"
 
-	OplogContainerObject = OplogContinerWriter.Save();
+	OplogContainerObject = OplogContainerWriter.Save();
 
 	return OplogContainerObject;
 }
@@ -3575,9 +3570,6 @@ SaveOplog(CidStore&				 ChunkStore,
 	LooseLargeFiles.clear();
 	CreatedBlocks.clear();
 
-	//		RemoteProjectStore::Result Result = RemoteResult.ConvertResult();
-	//		Result.ElapsedSeconds			  = Timer.GetElapsedTimeMs() / 1000.0;
-
 	remotestore_impl::LogRemoteStoreStatsDetails(RemoteStore.GetStats());
 
 	remotestore_impl::ReportMessage(
@@ -3592,8 +3584,6 @@ SaveOplog(CidStore&				 ChunkStore,
 					Info.AttachmentsUploaded.load(),
 					NiceBytes(Info.AttachmentBytesUploaded.load()),
 					remotestore_impl::GetStats(RemoteStore.GetStats(), /*OptionalCacheStats*/ nullptr, TransferWallTimeMS)));
-
-	//	return Result;
 };
 
 RemoteProjectStore::Result
@@ -4340,8 +4330,19 @@ LoadOplog(LoadOplogContext&& Context)
 							Stopwatch Timer;
 							IoBuffer  TmpBuffer;
 							{
-								BasicFile TmpFile;
-								TmpFile.Open(TempFileName, BasicFile::Mode::kTruncate);
+								BasicFile		TmpFile;
+								std::error_code Ec;
+								TmpFile.Open(TempFileName, BasicFile::Mode::kTruncate, Ec);
+								if (Ec)
+								{
+									RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::InternalServerError),
+														  "Missing chunk",
+														  fmt::format("Failed to open temp file {} for chunked attachment {}",
+																	  TempFileName,
+																	  Chunked.RawHash));
+									return;
+								}
+								else
 								{
 									BasicFileWriter TmpWriter(TmpFile, 64u * 1024u);
 
@@ -4473,7 +4474,7 @@ LoadOplog(LoadOplogContext&& Context)
 						catch (const std::exception& Ex)
 						{
 							RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::InternalServerError),
-												  fmt::format("Failed to dechunck file {}", Chunked.RawHash),
+												  fmt::format("Failed to dechunk file {}", Chunked.RawHash),
 												  Ex.what());
 						}
 					},
@@ -4531,7 +4532,13 @@ LoadOplog(LoadOplogContext&& Context)
 		}
 		if (Result.ErrorCode == 0)
 		{
-			remotestore_impl::WriteOplogSection(Context.Oplog, OplogSection, Context.OptionalJobContext);
+			Result = remotestore_impl::WriteOplogSection(Context.Oplog, OplogSection, Context.OptionalJobContext);
+			if (Result.ErrorCode)
+			{
+				RemoteResult.SetError(Result.ErrorCode, Result.Reason, Result.Text);
+				remotestore_impl::ReportMessage(Context.OptionalJobContext,
+												fmt::format("Aborting ({}): {}", Result.ErrorCode, Result.Reason));
+			}
 		}
 	}
 
diff --git a/src/zenserver-test/projectstore-tests.cpp b/src/zenserver-test/projectstore-tests.cpp
index 51b307086..52ae937f5 100644
--- a/src/zenserver-test/projectstore-tests.cpp
+++ b/src/zenserver-test/projectstore-tests.cpp
@@ -575,7 +575,6 @@ TEST_CASE("project.remote")
 				{
 					Writer << "maxblocksize"sv << 3072u;
 					Writer << "maxchunkembedsize"sv << 1296u;
-					Writer << "maxchunksperblock"sv << 16u;
 					Writer << "chunkfilesizelimit"sv << 5u * 1024u;
 					Writer << "maxchunksperblock"sv << 16u;
 					Writer << "force"sv << false;