Skip to content

Commit 1d2fd81

Browse files
committed
Name the real exception in TaskGroup failure summaries
When a handler's TaskGroup crashed with an unexpected exception, _classify_exception_group returned str(eg), i.e. "unhandled errors in a TaskGroup (1 sub-exception)". That text is what every wrapper up to the caller and the streamed logs embed, so the actual error (a cp1252 UnicodeDecodeError on the Windows CI, issues #46/#47) was visible only in the ER's own log. - Flatten nested exception groups to their leaves before classifying, deduplicating by identity so an exception wrapped at several levels is listed once while distinct failures with equal text all show. - Summarize an unexpected leaf as "Type: message", falling back to repr when the message is empty. Known failures keep their own message verbatim, as a deeper layer already summarized them. - Treat asyncio.CancelledError as a benign cancellation and drop empty messages, so a message-less cancelled sibling neither counts as a crash nor leaves a trailing "; " in the summary. - Add test_exception_group_summary, including an end-to-end handler whose own TaskGroup raises.
1 parent 68866c7 commit 1d2fd81

2 files changed

Lines changed: 319 additions & 3 deletions

File tree

‎finecode_extension_runner/src/finecode_extension_runner/_services/run_action.py‎

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ def _is_cancellation(exc: BaseException) -> bool:
8181
"""True if *exc* signals a benign cancellation rather than a failure.
8282
8383
Recognizes:
84+
- ``asyncio.CancelledError`` — a task was cancelled. CPython's
85+
``asyncio.TaskGroup`` omits cancelled children from the group it
86+
raises, so this only matters for groups built elsewhere; there a
87+
CancelledError member must not be reported as a crash.
8488
- ilspclient.LspRequestCancelledError — a downstream LSP server
8589
cancelled a request.
8690
- code_action.ActionCancelledException — a handler cancelled
@@ -93,6 +97,7 @@ def _is_cancellation(exc: BaseException) -> bool:
9397
return isinstance(
9498
exc,
9599
(
100+
asyncio.CancelledError,
96101
ilspclient.LspRequestCancelledError,
97102
code_action.ActionCancelledException,
98103
ActionCancelledException,
@@ -124,6 +129,44 @@ def _is_known_failure(exc: BaseException) -> bool:
124129
)
125130

126131

132+
def _flatten_exception_group(eg: BaseExceptionGroup) -> list[BaseException]:
133+
"""Flatten a (possibly nested) exception group down to its leaf exceptions.
134+
135+
A TaskGroup's exception group nests rather than merges: a task that itself
136+
ran a TaskGroup (e.g. a dispatch handler's own ``asyncio.TaskGroup`` around
137+
sub-action runs) contributes its group as one member of the outer group.
138+
Classification and summarization want the leaves — the exceptions a human
139+
can act on.
140+
141+
A leaf reachable through several nested groups is returned once (by
142+
identity); distinct exceptions with equal text are all kept, so the
143+
summary still shows how many tasks failed.
144+
"""
145+
leaves: dict[int, BaseException] = {}
146+
147+
def _collect(group: BaseExceptionGroup) -> None:
148+
for sub in group.exceptions:
149+
if isinstance(sub, BaseExceptionGroup):
150+
_collect(sub)
151+
else:
152+
leaves.setdefault(id(sub), sub)
153+
154+
_collect(eg)
155+
return list(leaves.values())
156+
157+
158+
def _exception_summary(exc: BaseException) -> str:
159+
"""One-line description of *exc* in the classical ``Type: message`` form.
160+
161+
Falls back to ``repr`` when the exception carries no message text (an empty
162+
``str``), so the summary still names what was raised.
163+
"""
164+
text = str(exc).strip()
165+
if not text:
166+
text = repr(exc)
167+
return f"{type(exc).__name__}: {text}"
168+
169+
127170
def _classify_exception_group(eg: BaseExceptionGroup) -> tuple[bool, str]:
128171
"""Classify every sub-exception of a TaskGroup's ExceptionGroup.
129172
@@ -133,24 +176,35 @@ def _classify_exception_group(eg: BaseExceptionGroup) -> tuple[bool, str]:
133176
failure (``_is_known_failure``) is genuinely unexpected — this is the
134177
ER's one chance to log its full traceback, since callers only see the
135178
summarized message from here on.
179+
180+
The message names the underlying leaf exceptions in ``Type: message``
181+
form (``_flatten_exception_group`` recurses through nested groups first).
182+
An unexpected exception is not collapsed into the group's own summary —
183+
``str(eg)`` would hide it behind the opaque ``unhandled errors in a
184+
TaskGroup (N sub-exception)`` text — because that message is what
185+
propagates through every wrapper up to the caller and the streamed logs.
136186
"""
137187
msgs: list[str] = []
138188
has_unknown = False
139189
all_cancelled = True
140-
for sub in eg.exceptions:
190+
for sub in _flatten_exception_group(eg):
141191
if _is_cancellation(sub):
142192
msgs.append(getattr(sub, "message", None) or str(sub))
143193
elif _is_known_failure(sub):
144194
msgs.append(sub.message)
145195
all_cancelled = False
146196
else:
197+
msgs.append(_exception_summary(sub))
147198
has_unknown = True
148199
all_cancelled = False
200+
# Empty entries are message-less cancellations (``str(CancelledError())``
201+
# is "") — noise next to the messages that do say something.
202+
summary = "; ".join(m for m in msgs if m)
149203
if has_unknown:
150204
logger.error("Unhandled exception in action handler:")
151205
logger.exception(eg)
152-
return False, str(eg)
153-
return all_cancelled, "; ".join(msgs)
206+
return False, summary
207+
return all_cancelled, summary
154208

155209

156210
class StopWithResponse(Exception):
Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
from __future__ import annotations
2+
3+
import asyncio
4+
from pathlib import Path
5+
6+
import pytest
7+
from finecode_extension_api import code_action
8+
9+
from finecode_extension_runner._services import run_action as run_action_service
10+
from finecode_extension_runner.testing import handler_test_session
11+
12+
13+
def _decode_error() -> UnicodeDecodeError:
14+
"""A real cp1252 decode of UTF-8 content containing a curly quote — the
15+
exact shape a Windows box produces (issues #46/#47). The rendered text of
16+
``UnicodeDecodeError`` varies across CPython versions (3.14 formats byte
17+
ranges differently), so tests assert against the exception's own ``str()``
18+
rather than hardcoding the spelling.
19+
"""
20+
with pytest.raises(UnicodeDecodeError) as exc_info:
21+
"\u201d".encode("utf-8").decode("cp1252")
22+
return exc_info.value
23+
24+
25+
def _expected_summary(exc: BaseException) -> str:
26+
return f"{type(exc).__name__}: {exc}"
27+
28+
29+
def _classify(eg: BaseExceptionGroup) -> tuple[bool, str]:
30+
return run_action_service._classify_exception_group(eg)
31+
32+
33+
def test_unknown_exception_is_named_in_the_summary() -> None:
34+
"""An unexpected leaf exception must be surfaced as ``Type: message`` —
35+
not collapsed into the group's own opaque ``unhandled errors in a
36+
TaskGroup (N sub-exception)`` summary, which is what every wrapper above
37+
``_classify_exception_group`` embeds into its error string.
38+
"""
39+
error = _decode_error()
40+
eg = BaseExceptionGroup("unhandled errors in a TaskGroup", [error])
41+
42+
is_cancelled, message = _classify(eg)
43+
44+
assert is_cancelled is False
45+
assert message == _expected_summary(error)
46+
assert "unhandled errors in a TaskGroup" not in message
47+
48+
49+
def test_nested_groups_are_flattened_to_their_leaf_exceptions() -> None:
50+
"""A TaskGroup raises a group whose members can themselves be groups (a
51+
task that ran its own TaskGroup). The summary must name the deepest leaf,
52+
not the intermediate group wrappers.
53+
"""
54+
eg = BaseExceptionGroup(
55+
"outer",
56+
[BaseExceptionGroup("inner", [_decode_error()])],
57+
)
58+
59+
is_cancelled, message = _classify(eg)
60+
61+
assert is_cancelled is False
62+
assert message.startswith("UnicodeDecodeError: ")
63+
assert "inner" not in message
64+
65+
66+
def test_repeated_leaf_from_nested_wrapping_is_deduped() -> None:
67+
"""The same exception can surface more than once when nested groups wrap
68+
it at each level; the summary lists it once.
69+
"""
70+
error = _decode_error()
71+
eg = BaseExceptionGroup(
72+
"unhandled errors in a TaskGroup",
73+
[
74+
BaseExceptionGroup("branch-1", [error]),
75+
BaseExceptionGroup("branch-2", [error]),
76+
],
77+
)
78+
79+
is_cancelled, message = _classify(eg)
80+
81+
assert is_cancelled is False
82+
assert message == _expected_summary(error)
83+
assert message.count("UnicodeDecodeError") == 1
84+
85+
86+
def test_distinct_failures_with_equal_text_are_all_listed() -> None:
87+
"""Deduplication is by identity, not text: two tasks failing with the same
88+
message are two failures, and the summary shows both.
89+
"""
90+
first, second = _decode_error(), _decode_error()
91+
eg = BaseExceptionGroup("unhandled errors in a TaskGroup", [first, second])
92+
93+
is_cancelled, message = _classify(eg)
94+
95+
assert is_cancelled is False
96+
assert message == f"{_expected_summary(first)}; {_expected_summary(second)}"
97+
98+
99+
def test_real_task_group_omits_cancelled_siblings() -> None:
100+
"""CPython's TaskGroup leaves the siblings it cancelled out of the group it
101+
raises, so a real crash summarizes to just the failing leaf.
102+
"""
103+
104+
async def _boom() -> None:
105+
raise _decode_error()
106+
107+
async def _slow() -> None:
108+
await asyncio.sleep(10)
109+
110+
async def _run() -> None:
111+
async with asyncio.TaskGroup() as tg:
112+
tg.create_task(_boom())
113+
tg.create_task(_slow())
114+
115+
with pytest.raises(BaseExceptionGroup) as exc_info:
116+
asyncio.run(_run())
117+
118+
is_cancelled, message = _classify(exc_info.value)
119+
120+
assert is_cancelled is False
121+
assert message == _expected_summary(_decode_error())
122+
123+
124+
def test_unknown_leaf_with_cancelled_sibling_has_no_trailing_separator() -> None:
125+
"""A group built outside asyncio.TaskGroup may carry a message-less
126+
CancelledError (``str()`` is ""); it must neither leak ``; `` into the
127+
summary nor be reported as a second crash.
128+
"""
129+
error = _decode_error()
130+
eg = BaseExceptionGroup("g", [error, asyncio.CancelledError()])
131+
132+
is_cancelled, message = _classify(eg)
133+
134+
assert is_cancelled is False
135+
assert message == _expected_summary(error)
136+
137+
138+
def test_message_less_cancelled_error_group_still_counts_as_cancellation() -> None:
139+
"""A TaskGroup where only cancellations remain — e.g. every child was
140+
cancelled — is a benign cancellation (``all_cancelled``), not a crash.
141+
"""
142+
eg = BaseExceptionGroup("g", [asyncio.CancelledError()])
143+
144+
is_cancelled, _message = _classify(eg)
145+
146+
assert is_cancelled is True
147+
148+
149+
def test_known_failure_keeps_its_message_without_a_type_prefix() -> None:
150+
"""An already-classified failure (``_is_known_failure``) propagates its
151+
own message verbatim — it was already summarized by a deeper layer.
152+
"""
153+
known = run_action_service.ActionFailedException(
154+
"Running action handler 'pyrefly' failed(Run 0): boom"
155+
)
156+
eg = BaseExceptionGroup("g", [known])
157+
158+
is_cancelled, message = _classify(eg)
159+
160+
assert is_cancelled is False
161+
assert message == "Running action handler 'pyrefly' failed(Run 0): boom"
162+
assert "ActionFailedException" not in message
163+
164+
165+
def test_known_failure_and_unknown_leaf_are_both_listed() -> None:
166+
known = run_action_service.ActionFailedException("wrapped failure")
167+
eg = BaseExceptionGroup("g", [known, _decode_error()])
168+
169+
is_cancelled, message = _classify(eg)
170+
171+
assert is_cancelled is False
172+
assert message == f"wrapped failure; {_expected_summary(_decode_error())}"
173+
174+
175+
def test_known_failure_with_cancelled_sibling_has_no_trailing_separator() -> None:
176+
"""A recognized failure whose siblings the TaskGroup cancelled must render
177+
without trailing ``; `` noise from the message-less CancelledError.
178+
"""
179+
known = run_action_service.ActionFailedException("wrapped failure")
180+
eg = BaseExceptionGroup("g", [known, asyncio.CancelledError()])
181+
182+
is_cancelled, message = _classify(eg)
183+
184+
assert is_cancelled is False
185+
assert message == "wrapped failure"
186+
187+
188+
def test_app_level_cancellation_is_still_all_cancelled() -> None:
189+
eg = BaseExceptionGroup(
190+
"g",
191+
[run_action_service.ActionCancelledException("cancelled by pyrefly")],
192+
)
193+
194+
is_cancelled, message = _classify(eg)
195+
196+
assert is_cancelled is True
197+
assert message == "cancelled by pyrefly"
198+
199+
200+
# ---------------------------------------------------------------------------
201+
# End-to-end: the CI shape. A handler whose own TaskGroup crashes with an
202+
# unexpected exception used to surface as "Running action handler ... failed(
203+
# Run N): unhandled errors in a TaskGroup (1 sub-exception)" — the real
204+
# exception was buried in the ER log only.
205+
# ---------------------------------------------------------------------------
206+
207+
208+
class _ExceptionSummaryTestAction(code_action.Action):
209+
"""Uses the base Action's default payload/run-context/result types — the
210+
tests only care about how the raised exception is summarized."""
211+
212+
213+
class _TaskGroupUnicodeErrorHandler(
214+
code_action.ActionHandler[
215+
_ExceptionSummaryTestAction, code_action.ActionHandlerConfig
216+
]
217+
):
218+
async def run(
219+
self,
220+
payload: code_action.RunActionPayload,
221+
run_context: code_action.RunActionContext,
222+
) -> code_action.RunActionResult:
223+
async with asyncio.TaskGroup() as tg:
224+
tg.create_task(_raise_decode_error())
225+
return None # pragma: no cover - unreachable, task group always raises
226+
227+
228+
async def _raise_decode_error() -> None:
229+
raise _decode_error()
230+
231+
232+
def _single_handler_action(action_name: str, handler_cls: type) -> dict[str, dict]:
233+
handler_source = f"{handler_cls.__module__}.{handler_cls.__qualname__}"
234+
action_source = (
235+
f"{_ExceptionSummaryTestAction.__module__}."
236+
f"{_ExceptionSummaryTestAction.__qualname__}"
237+
)
238+
return {
239+
action_name: {
240+
"source": action_source,
241+
"handlers": [{"name": handler_cls.__name__, "source": handler_source}],
242+
}
243+
}
244+
245+
246+
@pytest.mark.asyncio
247+
async def test_task_group_crash_in_handler_surfaces_the_real_exception(
248+
tmp_path: Path,
249+
) -> None:
250+
"""Regression test for the Windows CI `inspect_code` failure (issue #47):
251+
when an unexpected exception escapes a handler through a TaskGroup, the
252+
propagated ActionFailedException message must name the exception instead
253+
of the opaque group summary.
254+
"""
255+
actions = _single_handler_action("boom_action", _TaskGroupUnicodeErrorHandler)
256+
async with handler_test_session(project_dir=tmp_path, actions=actions) as session:
257+
with pytest.raises(run_action_service.ActionFailedException) as exc_info:
258+
await session.run_action("boom_action")
259+
260+
error = _decode_error()
261+
assert _expected_summary(error) in exc_info.value.message
262+
assert "unhandled errors in a TaskGroup" not in exc_info.value.message

0 commit comments

Comments
 (0)