fix: preserve finalizer reconciliation errors
This commit is contained in:
+18
-2
@@ -468,12 +468,16 @@ def finalize_with_progress(
|
||||
pass
|
||||
|
||||
move_completed = False
|
||||
finalizer_error: BaseException | None = None
|
||||
finalizer_traceback = None
|
||||
try:
|
||||
with log_path.open("a", encoding="utf-8") as log_file:
|
||||
emit(ProgressEvent(ProgressPhase.MOVING, 0), log_file)
|
||||
try:
|
||||
destination = finalizer(plan, roots, validation)
|
||||
except BaseException as error:
|
||||
finalizer_error = error
|
||||
finalizer_traceback = error.__traceback__
|
||||
failure_event = ProgressEvent(
|
||||
ProgressPhase.MOVING,
|
||||
max(0.0, clock() - started),
|
||||
@@ -505,8 +509,20 @@ def finalize_with_progress(
|
||||
completion_event.elapsed_seconds,
|
||||
after_completed_move=True,
|
||||
)
|
||||
except Exception as progress_error:
|
||||
if not move_completed:
|
||||
except BaseException as progress_error:
|
||||
if finalizer_error is not None:
|
||||
if progress_error is finalizer_error:
|
||||
raise
|
||||
message = (
|
||||
"progress reporting failed while reporting failed file move: "
|
||||
f"{progress_error}"
|
||||
)
|
||||
try:
|
||||
on_warning(message)
|
||||
except Exception:
|
||||
pass
|
||||
raise finalizer_error.with_traceback(finalizer_traceback)
|
||||
if not move_completed or not isinstance(progress_error, Exception):
|
||||
raise
|
||||
message = (
|
||||
"progress reporting failed after completed file move: "
|
||||
|
||||
@@ -22,6 +22,7 @@ from app.tui import (
|
||||
confirm_exact,
|
||||
finalize_with_progress,
|
||||
)
|
||||
from app.workflow import WorkflowError
|
||||
|
||||
|
||||
class Session:
|
||||
@@ -431,6 +432,52 @@ def test_progress_log_open_failure_prevents_finalizer(
|
||||
assert finalizer_calls == []
|
||||
|
||||
|
||||
def test_log_close_failure_does_not_mask_finalizer_reconciliation_error(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
original = WorkflowError("manual reconciliation required")
|
||||
warnings: list[str] = []
|
||||
|
||||
class FailingCloseLog:
|
||||
def __enter__(self) -> "FailingCloseLog":
|
||||
return self
|
||||
|
||||
def __exit__(self, *args: object) -> None:
|
||||
raise OSError("progress log close failed")
|
||||
|
||||
def write(self, value: str) -> int:
|
||||
return len(value)
|
||||
|
||||
def flush(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(Path, "open", lambda *args, **kwargs: FailingCloseLog())
|
||||
plan = _plan(
|
||||
_book(tmp_path / "pending" / "Book"),
|
||||
BookMetadata("Book", "Author"),
|
||||
(Chapter(0, "One"),),
|
||||
_settings(tmp_path),
|
||||
)
|
||||
|
||||
with pytest.raises(WorkflowError) as caught:
|
||||
finalize_with_progress(
|
||||
plan,
|
||||
_settings(tmp_path).roots,
|
||||
ValidationResult(True),
|
||||
lambda plan, roots, validation: (_ for _ in ()).throw(original),
|
||||
tmp_path / "state" / "logs" / "Book.log",
|
||||
lambda event: None,
|
||||
on_warning=warnings.append,
|
||||
)
|
||||
|
||||
assert caught.value is original
|
||||
assert str(caught.value) == "manual reconciliation required"
|
||||
assert warnings == [
|
||||
"progress reporting failed while reporting failed file move: "
|
||||
"progress log close failed"
|
||||
]
|
||||
|
||||
|
||||
def test_failed_validation_never_emits_moving_or_calls_finalizer(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
|
||||
Reference in New Issue
Block a user