diff --git a/pyrefly-baseline.json b/pyrefly-baseline.json index ef795fd..dc4a188 100644 --- a/pyrefly-baseline.json +++ b/pyrefly-baseline.json @@ -144,90 +144,6 @@ "concise_description": "Argument `int | ndarray` is not assignable to parameter `total` with type `int` in function `jamma.core.progress.progress_iterator`", "severity": "error" }, - { - "line": 178, - "column": 23, - "stop_line": 178, - "stop_column": 38, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `tell`", - "concise_description": "Object of class `NoneType` has no attribute `tell`", - "severity": "error" - }, - { - "line": 179, - "column": 17, - "stop_line": 179, - "stop_column": 33, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `write`", - "concise_description": "Object of class `NoneType` has no attribute `write`", - "severity": "error" - }, - { - "line": 180, - "column": 17, - "stop_line": 180, - "stop_column": 33, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `flush`", - "concise_description": "Object of class `NoneType` has no attribute `flush`", - "severity": "error" - }, - { - "line": 186, - "column": 21, - "stop_line": 186, - "stop_column": 36, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `seek`", - "concise_description": "Object of class `NoneType` has no attribute `seek`", - "severity": "error" - }, - { - "line": 186, - "column": 37, - "stop_line": 186, - "stop_column": 40, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "unbound-name", - "description": "`pos` is uninitialized", - "concise_description": "`pos` is uninitialized", - "severity": "error" - }, - { - "line": 187, - "column": 21, - "stop_line": 187, - "stop_column": 40, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "missing-attribute", - "description": "Object of class `NoneType` has no attribute `truncate`", - "concise_description": "Object of class `NoneType` has no attribute `truncate`", - "severity": "error" - }, - { - "line": 191, - "column": 28, - "stop_line": 191, - "stop_column": 31, - "path": "src/jamma/lmm/io.py", - "code": -2, - "name": "unbound-name", - "description": "`pos` is uninitialized", - "concise_description": "`pos` is uninitialized", - "severity": "error" - }, { "line": 1068, "column": 38, diff --git a/src/jamma/lmm/io.py b/src/jamma/lmm/io.py index 5dba827..6450da2 100644 --- a/src/jamma/lmm/io.py +++ b/src/jamma/lmm/io.py @@ -170,21 +170,34 @@ def _write_buf(self, buf: str, count: int) -> None: count: Number of logical results in the buffer. Raises: - OSError: After exhausting retries on write failure. + RuntimeError: If the writer is not open. + OSError: After exhausting retries on write failure, or immediately + if the rollback position cannot be read. """ + handle = self._file + if handle is None: + raise RuntimeError("Writer not opened. Use as context manager.") + + # Read the rollback position once, outside the retry loop. It is the + # same on every attempt, since a failed write is truncated back to it. + # Taking it inside the try left `pos` unbound whenever tell() itself + # raised, so the rollback below raised UnboundLocalError instead -- + # not an OSError, so it bypassed the retry, the rollback and the + # partial-file cleanup, and escaped this method's documented contract. + pos = handle.tell() + last_error: OSError | None = None for attempt in range(1 + len(_RETRY_BACKOFF)): try: - pos = self._file.tell() - self._file.write(buf) - self._file.flush() + handle.write(buf) + handle.flush() self._count += count return except OSError as e: last_error = e try: - self._file.seek(pos) - self._file.truncate() + handle.seek(pos) + handle.truncate() except OSError as seek_err: logger.warning( f"Failed to rollback partial write at position " diff --git a/tests/test_incremental_writer.py b/tests/test_incremental_writer.py index 8b19572..275af86 100644 --- a/tests/test_incremental_writer.py +++ b/tests/test_incremental_writer.py @@ -633,3 +633,36 @@ def failing_seek(pos): assert not output_path.exists(), ( "Partial output file should be deleted when EPERM + rollback both fail" ) + + @pytest.mark.tier0 + def test_write_buf_tell_failure_surfaces_as_oserror( + self, tmp_path: Path, sample_result: AssocResult + ): + """An OSError from tell() propagates as OSError, not UnboundLocalError. + + tell() supplies the rollback position. Reading it inside the retry + try-block left the position unbound when tell() itself failed, so the + rollback in the except-handler raised UnboundLocalError -- which is not + an OSError, so it escaped write()'s documented contract and skipped the + partial-file cleanup entirely. + """ + output_path = tmp_path / "test.assoc.txt" + + with patch("jamma.lmm.io.time.sleep"): + with pytest.raises(OSError, match="Illegal seek"): + with IncrementalAssocWriter(output_path) as writer: + + def failing_tell(): + err = OSError("Illegal seek") + err.errno = errno.ESPIPE + raise err + + handle = writer._file + assert handle is not None + handle.tell = failing_tell + + writer.write(sample_result) + + assert not output_path.exists(), ( + "Partial output file should be deleted when tell() fails" + )