From fb975fb09865e5372264fb132e0ada4453d1ef91 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 11:53:24 -0700 Subject: [PATCH] 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=` 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. --- plugins/platforms/slack/adapter.py | 25 +++++-------------- .../test_empty_sentinel_rewrite_copy.py | 20 ++------------- tests/gateway/test_slack_send_retry.py | 2 ++ 3 files changed, 10 insertions(+), 37 deletions(-) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 51deba7956..1d1fe52fbf 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -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: diff --git a/tests/gateway/test_empty_sentinel_rewrite_copy.py b/tests/gateway/test_empty_sentinel_rewrite_copy.py index ea39f35887..b8b6931035 100644 --- a/tests/gateway/test_empty_sentinel_rewrite_copy.py +++ b/tests/gateway/test_empty_sentinel_rewrite_copy.py @@ -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 diff --git a/tests/gateway/test_slack_send_retry.py b/tests/gateway/test_slack_send_retry.py index 84a36aaf26..c3f3d676b5 100644 --- a/tests/gateway/test_slack_send_retry.py +++ b/tests/gateway/test_slack_send_retry.py @@ -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"}