Report a bad ini core value as a handled error - #4028
Conversation
The ini loader called self.to() unguarded, so a conversion failure on a [tox] core value escaped as a raw traceback. The TOML loader already wraps the same call and raises HandledError, so identical input gave a clean message in pyproject.toml but a traceback in tox.ini.
for more information, see https://pre-commit.ci
|
Reproduced this on Windows 11 / Python 3.14.7 with On On this branch at
Small observation rather than a request: the ini path reports Ran the commands above myself; the write-up was drafted with AI assistance. |
CAOShurong
left a comment
There was a problem hiding this comment.
Verified end-to-end on the current head (4616771), Windows / Python 3.13, tox installed from the exact head SHA into an isolated --target dir:
- baseline + in tox.ini: raw traceback (unhandled), exit 1 — reproduced;
- this PR head, same ini: , no traceback, and the same exit code and message shape as the existing TOML path ( on both main and this head, unchanged), so the "matching the TOML loader behavior" claim holds exactly;
- the HandledError/Skip passthrough with broad wrap only for core-section values (env values keep their own guards) is the right scope.
One cosmetic note, not a blocker: the ini path prefixes the key with while the TOML path uses — inherent to each loader's section name, just worth knowing when grepping logs.
CAOShurong
left a comment
There was a problem hiding this comment.
Verified end-to-end on the current head (4616771), Windows / Python 3.13, tox installed from the exact head SHA into an isolated --target dir:
- baseline tox @ main with min_version = notaversion in tox.ini: raw packaging.version.InvalidVersion traceback (unhandled), exit 1 — reproduced;
- this PR head, same ini: "ROOT: HandledError| failed to load tox.min_version: Invalid version: 'notaversion'" — no traceback, and the same exit code and message shape as the existing TOML path ("failed to load core.min_version: ...", identical on main and this head), so the "matching the TOML loader behavior" claim holds exactly;
- the HandledError/Skip passthrough with the broad Exception wrap applied only to core-section values (environment values keep their own guards) is the right scope.
One cosmetic note, not a blocker: the ini path prefixes the key with "tox." while the TOML path uses "core." — inherent to each loader's section name, just worth knowing when grepping logs.
A bad value in the ini
[tox]core section exits with a raw traceback, while the same value inpyproject.tomlis already reported properly.The TOML path, same input, unchanged by this PR:
After:
requires = ===bad!!!andenv_list = {py39,py310(unbalanced brace) were tracebacks too, and are now handled the same way.The fix
IniLoader.buildcalledself.to(...)unguarded.TomlLoader.buildalready wraps the identical call and re-raises asHandledError, so this mirrors it, including lettingHandledErrorandSkipthrough untouched.Why the guard is narrow
The wrap only applies to the core section. My first attempt covered every section and broke 6 existing tests, because two paths deliberately let raw exceptions through:
tox crenders the exception inline as# Exception: ...(test_config_bad_dict,test_config_bad_booland friends)conf is NoneThose are existing behaviour I did not want to change, so the condition is
conf is None or args.env_name is not Noneto bypass. Core values are the case with no handling at all, because they are loaded before any guard is in place.Verification
test_ini_core_bad_value_is_handled_errorintests/config/source/test_discover.py, parametrised overmin_version,requiresandenv_list. It asserts no unhandled exception leaks, so it fails on the actual symptom rather than on a message string.Reverting only the guard condition to always bypass fails it as an assertion:
pytest tests/config/is 6644 passed, 2 skipped.tests/config/cli/test_argcomplete.pyis excluded becauseargcompleteis not installed here; it fails to collect identically on a clean checkout.Disclosure: written with AI assistance (Claude Code). I reproduced the traceback and the TOML contrast at the CLI, confirmed the narrow guard is required by removing it and watching 6 tests break, and ran the mutation check myself.