Skip to content

Raise MessageParseError (not TypeError) on malformed field types #988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Zeffut wants to merge 1 commit into from
+61 −0
Closed

Raise MessageParseError (not TypeError) on malformed field types #988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/claude_agent_sdk/_internal/message_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ def parse_message(data: dict[str, Any]) -> Message | None:
raise MessageParseError(
f"Missing required field in user message: {e}", data
) from e
except TypeError as e:
raise MessageParseError(
f"Malformed field in user message: {e}", data
) from e
Comment on lines +125 to +128
Copy link
Copy Markdown
Author

@Zeffut Zeffut May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair concern on TypeError breadth. The scope here is intentionally narrow: each except TypeError sits inside the same small try block that already catches KeyError for the same parsing step, so it only wraps the dict subscripting / unpacking of CLI payload fields — not arbitrary downstream logic. This mirrors the existing KeyError -> MessageParseError pattern in the file (symmetric handling for the two ways a malformed payload can fail). Pre-validating shapes per branch would work too, but it would expand the diff considerably and duplicate checks the dataclass constructors already perform. Re-raising as MessageParseError with the original data preserves both the original TypeError (via 'from e') and the malformed payload for debugging, so genuine programmer errors remain inspectable in tracebacks.


case "assistant":
try:
Expand Down Expand Up @@ -184,6 +188,10 @@ def parse_message(data: dict[str, Any]) -> Message | None:
raise MessageParseError(
f"Missing required field in assistant message: {e}", data
) from e
except TypeError as e:
raise MessageParseError(
f"Malformed field in assistant message: {e}", data
) from e

case "system":
try:
Expand Down Expand Up @@ -242,6 +250,10 @@ def parse_message(data: dict[str, Any]) -> Message | None:
raise MessageParseError(
f"Missing required field in system message: {e}", data
) from e
except TypeError as e:
raise MessageParseError(
f"Malformed field in system message: {e}", data
) from e
Comment on lines +253 to +256
Copy link
Copy Markdown
Author

@Zeffut Zeffut May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point on test coverage parity. The three added regression tests target the originally reported rate_limit_event regression plus the two most commonly malformed branches (user/assistant, since their message field is what CLI payloads vary on most). The system/result/stream_event branches are wrapped symmetrically for defense-in-depth (the same TypeError -> MessageParseError shape, with original data attached), so behaviorally they are covered by the same contract the existing tests assert. Happy to extend coverage to those branches too if the maintainers prefer one test per branch — the 753-pass suite stays green either way.


case "result":
try:
Expand Down Expand Up @@ -275,6 +287,10 @@ def parse_message(data: dict[str, Any]) -> Message | None:
raise MessageParseError(
f"Missing required field in result message: {e}", data
) from e
except TypeError as e:
raise MessageParseError(
f"Malformed field in result message: {e}", data
) from e

case "stream_event":
try:
Expand All @@ -288,6 +304,10 @@ def parse_message(data: dict[str, Any]) -> Message | None:
raise MessageParseError(
f"Missing required field in stream_event message: {e}", data
) from e
except TypeError as e:
raise MessageParseError(
f"Malformed field in stream_event message: {e}", data
) from e

case "rate_limit_event":
try:
Expand All @@ -310,6 +330,10 @@ def parse_message(data: dict[str, Any]) -> Message | None:
raise MessageParseError(
f"Missing required field in rate_limit_event message: {e}", data
) from e
except TypeError as e:
raise MessageParseError(
f"Malformed field in rate_limit_event message: {e}", data
) from e

case _:
# Forward-compatible: skip unrecognized message types so newer
Expand Down
37 changes: 37 additions & 0 deletions tests/test_message_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,43 @@ def test_message_parse_error_contains_data(self):
parse_message(data)
assert exc_info.value.data == data

def test_parse_rate_limit_event_with_non_dict_info(self):
"""Malformed rate_limit_info (non-dict) raises MessageParseError, not TypeError.

A buggy or older CLI may emit ``rate_limit_info`` as a non-dict (e.g.
``None`` or a string). Such payloads must surface as
:class:`MessageParseError` like every other malformed-message case;
a raw ``TypeError`` would crash the parser loop and lose the rest
of the stream.
"""
for info_value in (None, "oops", 42):
data = {
"type": "rate_limit_event",
"rate_limit_info": info_value,
"uuid": "abc",
"session_id": "sess",
}
with pytest.raises(MessageParseError) as exc_info:
parse_message(data)
assert "rate_limit_event message" in str(exc_info.value)
assert exc_info.value.data == data

def test_parse_user_message_with_non_dict_message(self):
"""Malformed user message field (non-dict) raises MessageParseError."""
data = {"type": "user", "message": None}
with pytest.raises(MessageParseError) as exc_info:
parse_message(data)
assert "user message" in str(exc_info.value)
assert exc_info.value.data == data

def test_parse_assistant_message_with_non_dict_message(self):
"""Malformed assistant message field (non-dict) raises MessageParseError."""
data = {"type": "assistant", "message": None}
with pytest.raises(MessageParseError) as exc_info:
parse_message(data)
assert "assistant message" in str(exc_info.value)
assert exc_info.value.data == data

def test_parse_assistant_message_without_error(self):
"""Test that assistant message without error has error=None."""
data = {
Expand Down