From 2b33e6f9a445e15df3b6bd44889be005d459c60e Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Mon, 27 Jul 2026 01:01:03 -0600 Subject: [PATCH] Preserve real parse diagnostics instead of the generic section wrapper Fixes #12. The two generic section loops caught every exception and replaced it with "Error reading section %r", so a precise ParseError raised deep in a reader ("Unterminated string") was invisible to anyone showing str(e), and genuine internal bugs were laundered into what looked like a file-format complaint. - Both section loops now re-raise ParseError untouched, and include repr() of any other exception when wrapping it, so an internal error is still identifiable as one. - load_smaug_vnum_section / load_swr_vnum_section sliced instead of indexed when peeking at the character after '#', so a file ending in '#' ends the section instead of raising IndexError. - CircleAreaFile.read_record_header raises a located ParseError on a non-numeric record header instead of letting int() raise a bare ValueError. test_error_reporting.py covers all four behaviours; all six tests fail on the previous code. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PQh6Vypbkqk4vnqPUE8yTY --- area_reader/__init__.py | 21 ++++++++---- test_error_reporting.py | 71 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 test_error_reporting.py diff --git a/area_reader/__init__.py b/area_reader/__init__.py index b171482..0a6b7c5 100644 --- a/area_reader/__init__.py +++ b/area_reader/__init__.py @@ -335,8 +335,10 @@ def load_sections(self): logger.info("Processing section %s" % section_name) try: readers[section_name]() - except Exception: - self.parse_fail("Error reading section %r" % section_name) + except ParseError: + raise + except Exception as error: + self.parse_fail("Error reading section %r: %r" % (section_name, error)) def load_mobprogs(self): while True: @@ -1944,8 +1946,10 @@ def load_sections(self): logger.info("Processing section %s" % section_name) try: reader() - except Exception: - self.parse_fail("Error reading section %r" % section_name) + except ParseError: + raise + except Exception as error: + self.parse_fail("Error reading section %r: %r" % (section_name, error)) def read_area_metadata(self): self.area.name = self.read_string() @@ -1999,7 +2003,7 @@ def load_smaug_vnum_section(self, section_object_type): break if self.current_char != '#': self.parse_fail("Expected # got %s" % self.current_char) - next_char = self.data[self.index + 1] + next_char = self.data[self.index + 1:self.index + 2] if not (next_char.isdigit() or next_char == '-'): break vnum = self.read_vnum() @@ -2098,7 +2102,7 @@ def load_swr_vnum_section(self, section_object_type): break if self.current_char != '#': self.parse_fail("Expected # got %s" % self.current_char) - next_char = self.data[self.index + 1] + next_char = self.data[self.index + 1:self.index + 2] if not (next_char.isdigit() or next_char == '-'): break vnum = self.read_vnum() @@ -2843,7 +2847,10 @@ def read_record_header(self): self.index += 1 if self.current_char == '~': self.index += 1 - return int(token) + try: + return int(token) + except ValueError: + self.parse_fail("Expected numeric record header, got %r" % token) def read_int_list(self): line = self.read_line().strip() diff --git a/test_error_reporting.py b/test_error_reporting.py new file mode 100644 index 0000000..c358359 --- /dev/null +++ b/test_error_reporting.py @@ -0,0 +1,71 @@ +"""Regression tests for issue #12: section readers must not hide the real diagnostic.""" + +from pathlib import Path + +import pytest + +import area_reader + + +def write_area(tmp_path: Path, text: str, name: str = "broken.are") -> Path: + path = tmp_path / name + path.write_text(text, encoding="latin-1") + return path + + +def test_rom_section_error_keeps_the_specific_parse_message(tmp_path): + path = write_area(tmp_path, "#MOBILES\n#3000\nguard~\nA guard\n") + reader = area_reader.RomAreaFile(path) + + with pytest.raises(area_reader.ParseError) as caught: + reader.load_sections() + + assert "Unterminated string" in str(caught.value) + assert "Error reading section" not in str(caught.value) + + +def test_smaug_section_error_keeps_the_specific_parse_message(tmp_path): + path = write_area(tmp_path, "#MOBILES\n#3000\nguard~\nA guard\n") + reader = area_reader.SmaugAreaFile(path) + + with pytest.raises(area_reader.ParseError) as caught: + reader.load_sections() + + assert "Unterminated string" in str(caught.value) + assert "Error reading section" not in str(caught.value) + + +def test_non_parse_errors_are_reported_with_the_original_error(tmp_path, monkeypatch): + path = write_area(tmp_path, "#MOBILES\n#0\n\n#$\n") + reader = area_reader.RomAreaFile(path) + + def exploding_reader(): + raise RuntimeError("internal boom") + + monkeypatch.setattr(reader, "load_mobiles", exploding_reader) + + with pytest.raises(area_reader.ParseError) as caught: + reader.load_sections() + + assert "Error reading section 'mobiles'" in str(caught.value) + assert "internal boom" in str(caught.value) + + +@pytest.mark.parametrize("reader_class", ["SmaugAreaFile", "SwrAreaFile"]) +def test_vnum_section_tolerates_a_trailing_hash_at_end_of_file(tmp_path, reader_class): + path = write_area(tmp_path, "#MOBILES\n#", name="trailing.are") + reader = getattr(area_reader, reader_class)(path) + + reader.load_sections() + + assert reader.area.mobs == {} + + +def test_circle_record_header_rejects_a_non_numeric_vnum(tmp_path): + path = tmp_path / "broken.wld" + path.write_text("#\nname~\n", encoding="latin-1") + reader = area_reader.CircleAreaFile(tmp_path) + reader.open_circle_file(path) + + with pytest.raises(area_reader.ParseError, match="Expected numeric record header"): + reader.read_record_header()