Skip to content

Commit 6ccd363

Browse files
fix(adapters): align discord/linear/teams body extraction with working pattern
CodeRabbit flagged three real bugs in the discord/linear/teams `_get_request_body` implementations that the github/telegram/whatsapp fix didn't cover: - `str(await result if ... else result)` wrong-stringifies bytes returned from a callable `request.text()` (e.g. `str(b"...") -> "b'...'"`). - Non-callable `request.text` holding `bytes`/`bytearray` returns raw bytes to a `-> str` signature, typecheck lie + caller breaks. - `data` fallback missed `bytearray` (matches the earlier text/body bytes-vs-bytearray fix). - `read()` was only awaited via `iscoroutinefunction(raw_read)`, which misses wrapped/decorated methods that return an awaitable. Switched to `inspect.isawaitable(raw_result)` on the call result. Restructured all three to match the text-first shape used by the static adapters (github/telegram/whatsapp) and marked them `@staticmethod` — `self` was unused and consistency avoids future regressions. Tests: extended `test_request_body_extraction.py` matrix from 3 adapters to 6 and added `test_handles_sync_callable_body` for the previously unexercised sync body() branch. Dropped the broad `except ImportError: pass` guard — every adapter lazy-imports platform deps inside methods, so direct imports work and a broken adapter module now fails the suite instead of silently skipping (matches the "every test must fail when the code is wrong" rule). 54 parametrized cases (9 shapes × 6 adapters) — all passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a242e19 commit 6ccd363

4 files changed

Lines changed: 76 additions & 68 deletions

File tree

src/chat_sdk/adapters/discord/adapter.py

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,11 +1403,19 @@ async def _discord_fetch(
14031403
# Request/Response helpers (framework-agnostic)
14041404
# =========================================================================
14051405

1406-
async def _get_request_body(self, request: Any) -> str:
1406+
@staticmethod
1407+
async def _get_request_body(request: Any) -> str:
14071408
"""Extract the request body as a string."""
1408-
# `hasattr` narrows `Any` to `object` and kills downstream type info,
1409-
# so we use `getattr(..., default)` which preserves `Any`. The
1410-
# `await` below is safe because `inspect.isawaitable` guards it.
1409+
# `hasattr` narrows `Any` → `object` (not awaitable); using
1410+
# `getattr(..., None)` preserves `Any` for framework duck-typing.
1411+
# Handle both callable and non-callable `request.text`. Gating
1412+
# entry on callability would drop populated string attributes.
1413+
text_attr = getattr(request, "text", None)
1414+
if text_attr is not None:
1415+
if callable(text_attr):
1416+
result = text_attr()
1417+
text_attr = await result if inspect.isawaitable(result) else result
1418+
return text_attr.decode("utf-8") if isinstance(text_attr, (bytes, bytearray)) else str(text_attr)
14111419
body = getattr(request, "body", None)
14121420
if body is not None:
14131421
if callable(body):
@@ -1417,19 +1425,13 @@ async def _get_request_body(self, request: Any) -> str:
14171425
if inspect.isawaitable(body):
14181426
body = await body
14191427
if hasattr(body, "read"):
1420-
raw_read = body.read
1421-
raw = await raw_read() if inspect.iscoroutinefunction(raw_read) else raw_read()
1422-
return raw.decode("utf-8") if isinstance(raw, (bytes, bytearray)) else raw
1428+
raw_result = body.read()
1429+
raw = await raw_result if inspect.isawaitable(raw_result) else raw_result
1430+
return raw.decode("utf-8") if isinstance(raw, (bytes, bytearray)) else str(raw)
14231431
return body.decode("utf-8") if isinstance(body, (bytes, bytearray)) else str(body)
1424-
text_attr = getattr(request, "text", None)
1425-
if text_attr is not None:
1426-
if callable(text_attr):
1427-
result = text_attr()
1428-
return str(await result if inspect.isawaitable(result) else result)
1429-
return text_attr
14301432
data = getattr(request, "data", None)
14311433
if data is not None:
1432-
return data.decode("utf-8") if isinstance(data, bytes) else str(data)
1434+
return data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else str(data)
14331435
return ""
14341436

14351437
def _get_header(self, request: Any, name: str) -> str | None:

src/chat_sdk/adapters/linear/adapter.py

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -978,11 +978,19 @@ async def _graphql_query(
978978
# Request/Response helpers (framework-agnostic)
979979
# =========================================================================
980980

981-
async def _get_request_body(self, request: Any) -> str:
981+
@staticmethod
982+
async def _get_request_body(request: Any) -> str:
982983
"""Extract the request body as a string."""
983-
# `hasattr` narrows `Any` → `object` (which is not awaitable); using
984-
# `getattr(..., None)` preserves `Any` so the downstream `await` on
985-
# framework-returned coroutines type-checks.
984+
# `hasattr` narrows `Any` → `object` (not awaitable); using
985+
# `getattr(..., None)` preserves `Any` for framework duck-typing.
986+
# Handle both callable and non-callable `request.text`. Gating
987+
# entry on callability would drop populated string attributes.
988+
text_attr = getattr(request, "text", None)
989+
if text_attr is not None:
990+
if callable(text_attr):
991+
result = text_attr()
992+
text_attr = await result if inspect.isawaitable(result) else result
993+
return text_attr.decode("utf-8") if isinstance(text_attr, (bytes, bytearray)) else str(text_attr)
986994
body = getattr(request, "body", None)
987995
if body is not None:
988996
if callable(body):
@@ -992,19 +1000,13 @@ async def _get_request_body(self, request: Any) -> str:
9921000
if inspect.isawaitable(body):
9931001
body = await body
9941002
if hasattr(body, "read"):
995-
raw_read = body.read
996-
raw = await raw_read() if inspect.iscoroutinefunction(raw_read) else raw_read()
997-
return raw.decode("utf-8") if isinstance(raw, (bytes, bytearray)) else raw
1003+
raw_result = body.read()
1004+
raw = await raw_result if inspect.isawaitable(raw_result) else raw_result
1005+
return raw.decode("utf-8") if isinstance(raw, (bytes, bytearray)) else str(raw)
9981006
return body.decode("utf-8") if isinstance(body, (bytes, bytearray)) else str(body)
999-
text_attr = getattr(request, "text", None)
1000-
if text_attr is not None:
1001-
if callable(text_attr):
1002-
result = text_attr()
1003-
return str(await result if inspect.isawaitable(result) else result)
1004-
return text_attr
10051007
data = getattr(request, "data", None)
10061008
if data is not None:
1007-
return data.decode("utf-8") if isinstance(data, bytes) else str(data)
1009+
return data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else str(data)
10081010
return ""
10091011

10101012
def _get_header(self, request: Any, name: str) -> str | None:

src/chat_sdk/adapters/teams/adapter.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,10 +1768,19 @@ async def _verify_bot_framework_token(self, request: Any) -> Any | None:
17681768
# Request/Response helpers (framework-agnostic)
17691769
# =========================================================================
17701770

1771-
async def _get_request_body(self, request: Any) -> str:
1771+
@staticmethod
1772+
async def _get_request_body(request: Any) -> str:
17721773
"""Extract the request body as a string."""
17731774
# `hasattr` narrows `Any` → `object` (not awaitable); using
17741775
# `getattr(..., None)` preserves `Any` for framework duck-typing.
1776+
# Handle both callable and non-callable `request.text`. Gating
1777+
# entry on callability would drop populated string attributes.
1778+
text_attr = getattr(request, "text", None)
1779+
if text_attr is not None:
1780+
if callable(text_attr):
1781+
result = text_attr()
1782+
text_attr = await result if inspect.isawaitable(result) else result
1783+
return text_attr.decode("utf-8") if isinstance(text_attr, (bytes, bytearray)) else str(text_attr)
17751784
body = getattr(request, "body", None)
17761785
if body is not None:
17771786
if callable(body):
@@ -1781,19 +1790,13 @@ async def _get_request_body(self, request: Any) -> str:
17811790
if inspect.isawaitable(body):
17821791
body = await body
17831792
if hasattr(body, "read"):
1784-
raw_read = body.read
1785-
raw = await raw_read() if inspect.iscoroutinefunction(raw_read) else raw_read()
1786-
return raw.decode("utf-8") if isinstance(raw, (bytes, bytearray)) else raw
1793+
raw_result = body.read()
1794+
raw = await raw_result if inspect.isawaitable(raw_result) else raw_result
1795+
return raw.decode("utf-8") if isinstance(raw, (bytes, bytearray)) else str(raw)
17871796
return body.decode("utf-8") if isinstance(body, (bytes, bytearray)) else str(body)
1788-
text_attr = getattr(request, "text", None)
1789-
if text_attr is not None:
1790-
if callable(text_attr):
1791-
result = text_attr()
1792-
return str(await result if inspect.isawaitable(result) else result)
1793-
return text_attr
17941797
data = getattr(request, "data", None)
17951798
if data is not None:
1796-
return data.decode("utf-8") if isinstance(data, bytes) else str(data)
1799+
return data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else str(data)
17971800
return ""
17981801

17991802
def _get_header(self, request: Any, name: str) -> str | None:

tests/test_request_body_extraction.py

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -124,36 +124,26 @@ def __init__(self, body: bytearray) -> None:
124124
def _adapters() -> list[tuple[str, Any]]:
125125
"""Return (name, extractor) pairs for each adapter's body extraction.
126126
127-
Lazy imports because some adapters require platform deps (cryptography,
128-
pynacl). If a dep isn't installed, that adapter is skipped.
127+
Imported directly: every adapter lazy-imports its platform deps inside
128+
methods, so module-level import works in any environment the test runs
129+
in. A broken adapter module fails the suite — matching the repo rule
130+
"every test must fail when the code is wrong."
129131
"""
130-
result: list[tuple[str, Any]] = []
131-
132-
# GitHub (static method)
133-
try:
134-
from chat_sdk.adapters.github.adapter import GitHubAdapter
135-
136-
result.append(("github", GitHubAdapter._get_request_body))
137-
except ImportError:
138-
pass # Optional platform dep missing — skip this adapter's cases.
139-
140-
# Telegram (static method)
141-
try:
142-
from chat_sdk.adapters.telegram.adapter import TelegramAdapter
143-
144-
result.append(("telegram", TelegramAdapter._get_request_body))
145-
except ImportError:
146-
pass # Optional platform dep missing — skip this adapter's cases.
147-
148-
# WhatsApp (static method)
149-
try:
150-
from chat_sdk.adapters.whatsapp.adapter import WhatsAppAdapter
151-
152-
result.append(("whatsapp", WhatsAppAdapter._get_request_body))
153-
except ImportError:
154-
pass # Optional platform dep missing — skip this adapter's cases.
155-
156-
return result
132+
from chat_sdk.adapters.discord.adapter import DiscordAdapter
133+
from chat_sdk.adapters.github.adapter import GitHubAdapter
134+
from chat_sdk.adapters.linear.adapter import LinearAdapter
135+
from chat_sdk.adapters.teams.adapter import TeamsAdapter
136+
from chat_sdk.adapters.telegram.adapter import TelegramAdapter
137+
from chat_sdk.adapters.whatsapp.adapter import WhatsAppAdapter
138+
139+
return [
140+
("github", GitHubAdapter._get_request_body),
141+
("telegram", TelegramAdapter._get_request_body),
142+
("whatsapp", WhatsAppAdapter._get_request_body),
143+
("discord", DiscordAdapter._get_request_body),
144+
("linear", LinearAdapter._get_request_body),
145+
("teams", TeamsAdapter._get_request_body),
146+
]
157147

158148

159149
@pytest.mark.parametrize("name,extractor", _adapters())
@@ -207,6 +197,17 @@ async def test_handles_async_callable_body(name: str, extractor: Any) -> None:
207197
assert result == '{"ok": true}', f"{name} failed async body()"
208198

209199

200+
@pytest.mark.parametrize("name,extractor", _adapters())
201+
async def test_handles_sync_callable_body(name: str, extractor: Any) -> None:
202+
"""Sync `def body(self)` returning bytes is consumed directly —
203+
covers the callable-but-non-awaitable body branch. Without this,
204+
a regression that always awaits `body()` would only show up in
205+
async-framework use and silently pass the suite.
206+
"""
207+
result = await extractor(_SyncCallableBodyRequest(b'{"ok": true}'))
208+
assert result == '{"ok": true}', f"{name} failed sync body()"
209+
210+
210211
@pytest.mark.parametrize("name,extractor", _adapters())
211212
async def test_handles_bytes_body_attribute(name: str, extractor: Any) -> None:
212213
"""`request.body` as a bytes property is decoded as UTF-8."""

0 commit comments

Comments
 (0)