From a5926ebcfcb9816a36a50eedd9396a0790719831 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 5 Jul 2026 21:35:07 +0200 Subject: [PATCH] fix(pipeline): MP1b - remove dead try/catch in PakWriter.Dispose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review finding: Dispose()'s best-effort Finish() call was wrapped in a try/catch for InvalidOperationException that could never fire — the surrounding `if (!_finished)` guard already excludes the only condition under which Finish() throws that exception. Dead defensive code that LOOKS like it might be swallowing a real failure is exactly the kind of thing the no-workarounds policy asks to avoid; removed with a comment explaining why no catch is needed. Behavior unchanged; 43/43 Content.Tests still green. --- src/AcDream.Content/Pak/PakWriter.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/AcDream.Content/Pak/PakWriter.cs b/src/AcDream.Content/Pak/PakWriter.cs index 39618512..fe6e33bd 100644 --- a/src/AcDream.Content/Pak/PakWriter.cs +++ b/src/AcDream.Content/Pak/PakWriter.cs @@ -93,12 +93,12 @@ public sealed class PakWriter : IDisposable { } public void Dispose() { + // Best-effort finalize so a forgotten Finish() call doesn't leave a + // half-written file with a placeholder header claiming 0 entries + // while blobs are on disk. No try/catch needed: Finish() only throws + // when _finished is already true, which this guard already excludes. if (!_finished) { - // Best-effort finalize so a forgotten Finish() call doesn't leave a - // half-written file with a placeholder header claiming 0 entries - // while blobs are on disk — surfaced loudly instead via Finish()'s - // own invariants on next use, but here we simply flush what exists. - try { Finish(); } catch (InvalidOperationException) { /* already finished */ } + Finish(); } _stream.Dispose(); }