fix: slim the Slack edit-failure log to one line via the existing payload helper
Follow-up to the salvaged #111938 commit: `_slack_response_payload` already normalizes a SlackResponse/dict body, so the new `_slack_api_error_code` helper and the two-branch logger.error were redundant. One log line now always carries `api_error=<code|none>` so an HTTP 200 + ok=false failure (e.g. message_not_found) is readable without exc_info. Tests trimmed to one invariant per fix (session key on the response-ready line; API error code on the edit failure); the `session=unknown` fallback test was a change-detector.
This commit is contained in:
@@ -955,16 +955,6 @@ def _is_transient_transport_error(e: BaseException) -> bool:
|
||||
and not is_permanent_tls_error)
|
||||
|
||||
|
||||
def _slack_api_error_code(e: BaseException) -> Optional[str]:
|
||||
"""Return Slack's API error code when an SDK response exposes one."""
|
||||
response = getattr(e, "response", None)
|
||||
data = getattr(response, "data", None)
|
||||
if not isinstance(data, dict):
|
||||
return None
|
||||
error = data.get("error")
|
||||
return str(error) if error else None
|
||||
|
||||
|
||||
def _extra_or_env_flag_getter(key: str, env_var: str, *, strip: bool = False) -> Callable[..., bool]:
|
||||
"""Method factory: ``self._extra_or_env_flag(key, env_var, strip=strip)``."""
|
||||
|
||||
@@ -2357,15 +2347,12 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
message_id, chat_id, e, exc_info=True)
|
||||
return SendResult(
|
||||
success=False, error=str(e), retryable=True, error_kind="transient")
|
||||
api_error = _slack_api_error_code(e)
|
||||
if api_error:
|
||||
logger.error(
|
||||
"[Slack] API chat.update failure on message %s in channel %s: api_error=%s: %s",
|
||||
message_id, chat_id, api_error, e, exc_info=True)
|
||||
else:
|
||||
logger.error(
|
||||
"[Slack] Failed to edit message %s in channel %s: %s", message_id, chat_id, e,
|
||||
exc_info=True)
|
||||
# An HTTP 200 + ``ok=false`` reply raises too; its ``str()`` reads like a transport
|
||||
# failure ("status: 200") while the real cause is the body's error code.
|
||||
api_error = _slack_response_payload(getattr(e, "response", None)).get("error")
|
||||
logger.error(
|
||||
"[Slack] Failed to edit message %s in channel %s: api_error=%s: %s",
|
||||
message_id, chat_id, api_error or "none", e, exc_info=True)
|
||||
return SendResult(success=False, error=str(e))
|
||||
|
||||
async def delete_message(self, chat_id: str, message_id: str) -> bool:
|
||||
|
||||
@@ -39,6 +39,8 @@ async def test_empty_sentinel_rewrite_uses_the_shared_explanation_with_the_model
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_response_ready_log_includes_the_session_key(caplog):
|
||||
"""Per-conversation latency aggregation needs the session key on the completion line:
|
||||
``chat=`` alone merges every Slack thread of one channel (#111931)."""
|
||||
runner = _Runner()
|
||||
source = SimpleNamespace(chat_id="C1", platform=SimpleNamespace(value="slack"))
|
||||
|
||||
@@ -53,21 +55,3 @@ async def test_response_ready_log_includes_the_session_key(caplog):
|
||||
response_log = next(record.getMessage() for record in caplog.records
|
||||
if record.getMessage().startswith("response ready:"))
|
||||
assert "session=slack:C1:thread-123" in response_log
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_response_ready_log_uses_a_safe_session_fallback(caplog):
|
||||
runner = _Runner()
|
||||
source = SimpleNamespace(chat_id="C1", platform=SimpleNamespace(value="slack"))
|
||||
|
||||
with caplog.at_level("INFO", logger="gateway.run"):
|
||||
await runner._hmwa_shape_agent_response(
|
||||
{"final_response": "done", "messages": [], "api_calls": 1},
|
||||
source, history=[], session_entry=SimpleNamespace(session_id="s"),
|
||||
session_key=None, _quick_key=None, run_generation=0,
|
||||
_run_start_session_id="s", _platform_name="slack", _msg_start_time=0.0,
|
||||
)
|
||||
|
||||
response_log = next(record.getMessage() for record in caplog.records
|
||||
if record.getMessage().startswith("response ready:"))
|
||||
assert "session=unknown" in response_log
|
||||
|
||||
@@ -114,6 +114,8 @@ class TestSlackSendRetryable:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_api_failure_logs_the_slack_error_code(self, caplog):
|
||||
"""HTTP 200 + ok=false must name the body error code, not read as a transport failure
|
||||
(#111931)."""
|
||||
adapter = _make_adapter()
|
||||
error = _slack_api_error(200)
|
||||
error.response.data = {"ok": False, "error": "message_not_found"}
|
||||
|
||||
Reference in New Issue
Block a user