From 1589c522b96f45c80f3874109e6be450eea638f6 Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 10:59:03 -0400 Subject: [PATCH 1/2] Add internal exception handling to _send_error_notification Wrap the HTTP request in try/except so the function never raises. A notification helper should not propagate exceptions to its caller. Remove the now-redundant try/except wrapper in triage_issue(). Fixes #162 --- src/kai/triage.py | 45 ++++++++++++------------- tests/test_triage.py | 78 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 22 deletions(-) diff --git a/src/kai/triage.py b/src/kai/triage.py index b607054d..1cd80892 100644 --- a/src/kai/triage.py +++ b/src/kai/triage.py @@ -791,19 +791,12 @@ async def triage_issue( # notification, so just log and bail. if metadata is None: return - try: - await _send_error_notification( - metadata, - str(exc), - webhook_port, - webhook_secret, - ) - except Exception: - log.exception( - "Failed to send failure notification for %s#%d", - metadata.repo, - metadata.number, - ) + await _send_error_notification( + metadata, + str(exc), + webhook_port, + webhook_secret, + ) async def _send_error_notification( @@ -817,6 +810,7 @@ async def _send_error_notification( Called when the triage pipeline fails at any point. Sends a brief message so the user knows something went wrong and can check the logs. + Never raises - logs a warning on failure and returns. Args: metadata: Issue metadata for context. @@ -831,12 +825,19 @@ async def _send_error_notification( "X-Webhook-Secret": webhook_secret, } - async with ( - aiohttp.ClientSession() as session, - session.post(url, json={"text": text}, headers=headers) as resp, - ): - if resp.status != 200: - log.warning( - "send-message API returned %d for triage error notification", - resp.status, - ) + try: + async with ( + aiohttp.ClientSession() as session, + session.post(url, json={"text": text}, headers=headers) as resp, + ): + if resp.status != 200: + log.warning( + "send-message API returned %d for triage error notification", + resp.status, + ) + except Exception: + log.warning( + "Failed to send triage error notification for %s#%d", + metadata.repo, + metadata.number, + ) diff --git a/tests/test_triage.py b/tests/test_triage.py index 486373a6..ab24e053 100644 --- a/tests/test_triage.py +++ b/tests/test_triage.py @@ -11,6 +11,7 @@ IssueMetadata, _parse_triage_json, _sanitize_search_query, + _send_error_notification, apply_triage, build_triage_prompt, extract_issue_metadata, @@ -778,3 +779,80 @@ async def mock_exec(*args, **kwargs): mock_session.post.assert_called_once() body = mock_session.post.call_args[1]["json"] assert "failed" in body["text"].lower() + + +# ── _send_error_notification ────────────────────────────────────── + + +class TestSendErrorNotification: + """Verify _send_error_notification never raises.""" + + @pytest.mark.asyncio + async def test_does_not_raise_on_connection_error(self): + """Connection failure is caught and logged, not raised.""" + metadata = IssueMetadata( + repo="owner/repo", + number=42, + title="Test issue", + body="body", + author="user", + url="https://github.com/owner/repo/issues/42", + labels=[], + ) + + mock_session = AsyncMock() + mock_session.post = AsyncMock(side_effect=ConnectionError("refused")) + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with patch("kai.triage.aiohttp.ClientSession", return_value=mock_session): + # Should not raise + await _send_error_notification(metadata, "test error", 8080, "secret") + + @pytest.mark.asyncio + async def test_does_not_raise_on_timeout(self): + """Timeout is caught and logged, not raised.""" + + metadata = IssueMetadata( + repo="owner/repo", + number=42, + title="Test issue", + body="body", + author="user", + url="https://github.com/owner/repo/issues/42", + labels=[], + ) + + mock_session = AsyncMock() + mock_session.post = AsyncMock(side_effect=TimeoutError()) + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with patch("kai.triage.aiohttp.ClientSession", return_value=mock_session): + await _send_error_notification(metadata, "test error", 8080, "secret") + + @pytest.mark.asyncio + async def test_logs_warning_on_failure(self, caplog): + """A warning is logged when the notification fails.""" + metadata = IssueMetadata( + repo="owner/repo", + number=42, + title="Test issue", + body="body", + author="user", + url="https://github.com/owner/repo/issues/42", + labels=[], + ) + + mock_session = AsyncMock() + mock_session.post = AsyncMock(side_effect=RuntimeError("boom")) + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with ( + patch("kai.triage.aiohttp.ClientSession", return_value=mock_session), + caplog.at_level("WARNING", logger="kai.triage"), + ): + await _send_error_notification(metadata, "test error", 8080, "secret") + + assert "Failed to send triage error notification" in caplog.text From a846697bfff2d9912a734a6f4dbf01b861b3455f Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 11:01:18 -0400 Subject: [PATCH 2/2] Add exc_info=True to preserve exception traceback in log --- src/kai/triage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/kai/triage.py b/src/kai/triage.py index 1cd80892..aab48e97 100644 --- a/src/kai/triage.py +++ b/src/kai/triage.py @@ -840,4 +840,5 @@ async def _send_error_notification( "Failed to send triage error notification for %s#%d", metadata.repo, metadata.number, + exc_info=True, )