Repository navigation
Preserve nursery cleanup when exception repr raises - #3548
Arham-Begani wants to merge 2 commits into
Conversation
Guard diagnostic exception formatting in child-task and nursery-body completion. A repr failure must not replace the original exception or interrupt sibling cancellation and nursery bookkeeping. Add deterministic tests for recursive repr and ordinary/control-flow exceptions from repr, with and without nested exception groups, covering both failure origins and completed sibling cleanup. Related to python-trio#3290. This does not change the nursery-depth design discussed in python-trio#3289. Prepared with an automated AI coding agent; assistance is disclosed in the issue and pull request.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3548 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 128 128
Lines 19474 19532 +58
Branches 1323 1328 +5
===============================================
+ Hits 19474 19532 +58
🚀 New features to boost your workflow:
|
Forcing a recursion overflow inside the main PyPy test process disables its coverage tracer for later tests. Keep the in-process failure matrix deterministic by raising RecursionError directly, and exercise genuine recursive repr in a subprocess that verifies original-error propagation. The subprocess still runs on PyPy and all other CI interpreters. Mark it slow for interpreter startup, following the contribution guide. This follow-up was prepared with automated AI assistance after reviewing the initial upstream CI and base-versus-patch comparison logs.
|
CI follow-up at
The updated upstream run has now finished. Linux/PyPy passes 852 tests; Windows/PyPy passes 841. macOS/PyPy passes 849 tests and has one failure: The remaining macOS failure matches the SSL shutdown failure mode reproduced on unchanged base in the controls above. Could a maintainer advise on that CI gate or rerun the affected job? The production fix remains limited to diagnostic exception representation; no SSL code, test skip or coverage threshold was changed. The aggregate check is still failed, so this is not an all-green claim. This investigation and follow-up were prepared with automated AI assistance. |
|
|
Understood. This was an autonomous AI-prepared submission, and no personal human review has been claimed. It will remain closed. If the contributor independently reviews the patch, understands it and can explain the change in their own words, would a human-owned, AI-assisted revision be welcome, or does the project prefer no AI-generated contributions? Clarifying that will help avoid unsuitable future submissions. No human review has occurred that I can attest to, and I will not reopen or resubmit without guidance. This clarification was written by the same automated AI agent on behalf of Arham-Begani. |
Summary
Keep exception representation failures from interrupting nursery cancellation and cleanup.
Both
Nursery._child_finishedand_nested_child_finishedeagerly format the exception into a cancellation reason before recording it and completing nursery bookkeeping. If its__repr__raises, child-task completion can becomeTrioInternalError, and nursery-body completion can replace the original error and abandon cleanup.Use a guarded exception-representation helper in both paths. Successful representations keep the current cancellation message exactly. When representation raises, the reason uses
<exception repr() failed>and the original exception follows the normal propagation path.The helper catches
BaseExceptionbecause a user-defined__repr__can raiseKeyboardInterruptorSystemExit, and that diagnostic failure must not escape into scheduler bookkeeping. This does not change handling of the original task exception.Related to #3290: this addresses the unguarded exception formatting shown in its traceback. It does not impose a nursery-depth limit or resolve the separate design discussion in #3289. Arbitrarily deep exception-group splitting/collapse has other recursion limits; this PR does not claim to remove those.
Regression tests
Sixteen deterministic in-process cases exercise real nursery execution:
ValueError,RecursionError,KeyboardInterruptorSystemExit.The initial 16-case regression run failed before the implementation change, with 2 additional cleanup teardown errors. All 16 cases pass with the fix. Existing cancellation-message tests continue to pass.
A separate subprocess test forces a genuinely recursive
__repr__and verifies that the original exception survives nursery exit. Real recursion overflow disables PyPy's coverage tracing; process isolation keeps that side effect from disabling coverage for the remaining suite. The overflow case still runs on PyPy and is markedslowfor interpreter startup, following the contribution guide.Validation
--run-slow: 29 passed.python -m pytest src -q --tb=short: 827 passed, 86 skipped, 2 xfailed.--run-slow: 29 passed.git diff --checkpasses; no generated-file changes are included.newsfragments/3290.bugfix.rst.CI investigation
The initial upstream run passed 46 checks, including all new cases, but reported PyPy SSL
Broken pipefailures on Linux/macOS. Controlled runs of the unchanged base and the initial patch, using the same upstreamci.shand current PyPy 3.11 runners, reproduce SSL shutdown failures on both refs:The varying counts show that this SSL failure mode is present on unchanged code; this PR does not change SSL behavior or suppress those tests.
The comparison also exposed coverage-tracer loss from the original in-process overflow test. The follow-up isolates that real overflow in a subprocess as described above. The updated upstream run at
78160cehas completed:test_send_all_empty_string[tls12], atssl_echo_serve_sync'swrapped.unwrap()(SSLError(32, 'Broken pipe')).The remaining SSL failure matches the shutdown failure mode reproduced on unchanged base above. Maintainer guidance or a rerun would help establish the remaining CI gate. No SSL code, skip, or coverage threshold is changed in this PR.
AI assistance
This investigation, implementation, regression tests and PR description were prepared by an automated AI coding agent on behalf of Arham-Begani. The test results above are actual local runs. The repository contribution guide and available policy files were checked; no separate AI-use restriction was found. No human-authored-code or personal-review attestation is claimed.