fix(minimax-oauth): read streamed error bodies inside the client context + real-transport tests
Follow-ups on the salvaged bounded-read fix: - refresh flow: the non-200 branch reads a STREAMED body, which fails (ReadError/StreamClosed) once the httpx.Client context has exited — moved inside the context. Repro + regression test use a real socket server (MockTransport buffers in memory and cannot catch this). - truncation guard: >limit bodies end with ...[truncated] (mutation-checked against the is_stream_consumed fallback). - test mocks now model the streamed-read surface (is_stream_consumed, iter_bytes, client.send) so non-200 paths exercise the real bounded read.
This commit is contained in:
parent
94ef36a7f7
commit
219bb35c35
|
|
@ -8536,16 +8536,20 @@ def _refresh_minimax_oauth_state(
|
|||
"Accept": "application/json",
|
||||
},
|
||||
)
|
||||
if response.status_code != 200:
|
||||
body = _minimax_response_error_text(response)
|
||||
body_lower = body.lower()
|
||||
relogin = any(m in body_lower for m in
|
||||
("invalid_grant", "refresh_token_reused", "invalid_refresh_token"))
|
||||
raise AuthError(
|
||||
f"MiniMax OAuth refresh failed: {body or response.reason_phrase}",
|
||||
provider="minimax-oauth", code="refresh_failed",
|
||||
relogin_required=relogin,
|
||||
)
|
||||
# The non-200 branch reads a STREAMED body, so it must run while
|
||||
# the client is still open — iter_bytes() after the client context
|
||||
# closes raises (StreamClosed). The 200 path was already read by
|
||||
# _minimax_post_form, so response.json() below is safe outside.
|
||||
if response.status_code != 200:
|
||||
body = _minimax_response_error_text(response)
|
||||
body_lower = body.lower()
|
||||
relogin = any(m in body_lower for m in
|
||||
("invalid_grant", "refresh_token_reused", "invalid_refresh_token"))
|
||||
raise AuthError(
|
||||
f"MiniMax OAuth refresh failed: {body or response.reason_phrase}",
|
||||
provider="minimax-oauth", code="refresh_failed",
|
||||
relogin_required=relogin,
|
||||
)
|
||||
payload = response.json()
|
||||
if payload.get("status") != "success":
|
||||
raise AuthError(
|
||||
|
|
|
|||
|
|
@ -42,7 +42,13 @@ from hermes_cli.auth import (
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _make_httpx_response(status_code: int, body: dict | None = None, text: str = ""):
|
||||
"""Return a minimal mock that quacks like httpx.Response."""
|
||||
"""Return a minimal mock that quacks like httpx.Response.
|
||||
|
||||
Includes the streamed-read surface used by ``_minimax_post_form`` /
|
||||
``_minimax_response_error_text``: ``is_stream_consumed`` is False and
|
||||
``iter_bytes()`` yields the body/text bytes, so non-200 paths exercise
|
||||
the real bounded-read code instead of a truthy MagicMock attribute.
|
||||
"""
|
||||
resp = MagicMock()
|
||||
resp.status_code = status_code
|
||||
if body is not None:
|
||||
|
|
@ -52,6 +58,9 @@ def _make_httpx_response(status_code: int, body: dict | None = None, text: str =
|
|||
resp.json.side_effect = Exception("No body")
|
||||
resp.text = text
|
||||
resp.reason_phrase = "OK" if status_code == 200 else "Error"
|
||||
resp.is_stream_consumed = False
|
||||
resp.encoding = "utf-8"
|
||||
resp.iter_bytes.return_value = iter([resp.text.encode("utf-8")] if resp.text else [])
|
||||
return resp
|
||||
|
||||
|
||||
|
|
@ -128,6 +137,7 @@ def test_request_user_code_state_mismatch_raises():
|
|||
|
||||
client = MagicMock()
|
||||
client.post.return_value = mock_response
|
||||
client.send.return_value = mock_response
|
||||
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
_minimax_request_user_code(
|
||||
|
|
@ -387,6 +397,7 @@ def test_token_provider_refreshes_when_near_expiry():
|
|||
mock_instance.__enter__ = MagicMock(return_value=mock_instance)
|
||||
mock_instance.__exit__ = MagicMock(return_value=False)
|
||||
mock_instance.post.return_value = mock_resp
|
||||
mock_instance.send.return_value = mock_resp
|
||||
mock_client_class.return_value = mock_instance
|
||||
|
||||
token = provider()
|
||||
|
|
@ -441,6 +452,7 @@ def test_token_provider_quarantines_state_on_terminal_refresh():
|
|||
mock_instance.__enter__ = MagicMock(return_value=mock_instance)
|
||||
mock_instance.__exit__ = MagicMock(return_value=False)
|
||||
mock_instance.post.return_value = bad_resp
|
||||
mock_instance.send.return_value = bad_resp
|
||||
mock_client_class.return_value = mock_instance
|
||||
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
|
|
@ -475,3 +487,84 @@ def test_resolve_returns_callable_when_as_token_provider_true():
|
|||
assert creds["base_url"] == MINIMAX_OAUTH_GLOBAL_INFERENCE.rstrip("/")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bounded error-body reads (#56548 / PR #56549)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_refresh_error_body_bounded_and_readable_with_real_client():
|
||||
"""Refresh non-200 path over a REAL socket transport.
|
||||
|
||||
The error body is obtained via a streamed response; the bounded read
|
||||
must happen while the client context is still open. A real socket is
|
||||
required to bind this contract: closing the client tears the connection
|
||||
down, so a read after the ``with httpx.Client(...)`` block raises
|
||||
ReadError/StreamClosed. (MockTransport buffers in memory and would NOT
|
||||
catch the regression.)
|
||||
"""
|
||||
import http.server
|
||||
import socketserver
|
||||
import threading
|
||||
|
||||
import httpx
|
||||
|
||||
from hermes_cli.auth import _refresh_minimax_oauth_state
|
||||
|
||||
big_body = b"invalid_grant " + b"x" * (64 * 1024) # 64KB error body
|
||||
|
||||
class Handler(http.server.BaseHTTPRequestHandler):
|
||||
def do_POST(self):
|
||||
self.send_response(400)
|
||||
self.send_header("Content-Length", str(len(big_body)))
|
||||
self.end_headers()
|
||||
self.wfile.write(big_body)
|
||||
|
||||
def log_message(self, *args):
|
||||
pass
|
||||
|
||||
with socketserver.TCPServer(("127.0.0.1", 0), Handler) as server:
|
||||
port = server.server_address[1]
|
||||
thread = threading.Thread(target=server.serve_forever, daemon=True)
|
||||
thread.start()
|
||||
try:
|
||||
state = {
|
||||
"access_token": "expired",
|
||||
"refresh_token": "burned-rt",
|
||||
"portal_base_url": f"http://127.0.0.1:{port}",
|
||||
"client_id": MINIMAX_OAUTH_CLIENT_ID,
|
||||
"inference_base_url": MINIMAX_OAUTH_GLOBAL_INFERENCE,
|
||||
"expires_at": _past_iso(100),
|
||||
}
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
_refresh_minimax_oauth_state(state, force=True)
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
msg = str(exc_info.value)
|
||||
assert "invalid_grant" in msg
|
||||
assert exc_info.value.relogin_required is True
|
||||
# Bounded: 16KB limit + truncation marker, never the full 64KB body.
|
||||
assert len(msg) < 20 * 1024
|
||||
assert "...[truncated]" in msg
|
||||
|
||||
|
||||
def test_minimax_response_error_text_truncates_above_limit():
|
||||
"""Bodies above the 16KB bound are cut and marked truncated."""
|
||||
import httpx
|
||||
|
||||
from hermes_cli.auth import (
|
||||
_MINIMAX_OAUTH_ERROR_BODY_LIMIT,
|
||||
_minimax_response_error_text,
|
||||
)
|
||||
|
||||
big = "e" * (_MINIMAX_OAUTH_ERROR_BODY_LIMIT * 4)
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(500, text=big)
|
||||
|
||||
with httpx.Client(transport=httpx.MockTransport(handler)) as client:
|
||||
request = client.build_request("POST", "https://api.minimax.io/oauth/token")
|
||||
response = client.send(request, stream=True)
|
||||
text = _minimax_response_error_text(response)
|
||||
|
||||
assert text.endswith("...[truncated]")
|
||||
assert len(text) <= _MINIMAX_OAUTH_ERROR_BODY_LIMIT + len("...[truncated]")
|
||||
|
|
|
|||
Loading…
Reference in New Issue