Skip to content

Commit cc5adc4

Browse files
rodion-mclaude
andcommitted
Make tool error messages actionable for the model
Errors now carry an explicit `Retry: yes|no [(<window>)]` marker plus a numbered `Try: (1) ... (2) ... (3) ...` recovery hint, so the LLM can decide whether to retry and what to do next instead of looping on permanent failures or hallucinating fixes. - Refactor utils/errors.py around _ErrorTemplate records covering 401/403/404/422/429/500/502/503 with retryability flags and concrete wait windows. - Add `recovery_hints` parameter to handle_api_error for tool-specific per-status overrides; wire it into all 5 tools and remove the ad-hoc post-call 404 overrides in search.py and chat.py. - Each tool now points the model at the right next step on 404 (get_data_sources, conversation_id check, fresh codebase_search, etc.). - Expand test_error_handling.py from 10 to 17 tests, covering 422/502/ 503/timeout, recovery_hints overrides, and method-prefix propagation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6d9656f commit cc5adc4

7 files changed

Lines changed: 372 additions & 79 deletions

File tree

src/tests/test_error_handling.py

Lines changed: 145 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,80 +6,117 @@
66
from utils.errors import handle_api_error, format_data_source_names
77

88

9+
def _make_http_error(status_code: int, text: str = "") -> httpx.HTTPStatusError:
10+
response = MagicMock()
11+
response.status_code = status_code
12+
response.text = text
13+
return httpx.HTTPStatusError("", request=None, response=response)
14+
15+
16+
# ---------------------------------------------------------------------------
17+
# Status code mapping
18+
# ---------------------------------------------------------------------------
19+
920
@pytest.mark.asyncio
1021
async def test_handle_401_error():
11-
"""Test handling of 401 authentication errors."""
22+
"""401 errors must be tagged non-retryable and surface API key recovery steps."""
1223
ctx = MagicMock()
1324
ctx.error = AsyncMock()
1425

15-
# Create mock 401 response
16-
response = MagicMock()
17-
response.status_code = 401
18-
response.text = "Invalid token"
19-
20-
error = httpx.HTTPStatusError("", request=None, response=response)
21-
result = await handle_api_error(ctx, error, "test operation")
26+
result = await handle_api_error(ctx, _make_http_error(401, "Invalid token"), "test operation")
2227

2328
assert "Authentication error (401)" in result
2429
assert "Invalid API key" in result
30+
# New: explicit retry decision + actionable hint
31+
assert "Retry: no" in result
32+
assert "Try:" in result
33+
assert "CODEALIVE_API_KEY" in result
2534
ctx.error.assert_called_once()
2635

2736

2837
@pytest.mark.asyncio
2938
async def test_handle_404_error():
30-
"""Test handling of 404 not found errors."""
39+
"""404 errors must be non-retryable and point at get_data_sources."""
3140
ctx = MagicMock()
3241
ctx.error = AsyncMock()
3342

34-
response = MagicMock()
35-
response.status_code = 404
36-
response.text = "Resource not found"
37-
38-
error = httpx.HTTPStatusError("", request=None, response=response)
39-
result = await handle_api_error(ctx, error, "search")
43+
result = await handle_api_error(ctx, _make_http_error(404, "Resource not found"), "search")
4044

4145
assert "Not found error (404)" in result
46+
assert "Retry: no" in result
47+
assert "get_data_sources" in result
4248
ctx.error.assert_called_once()
4349

4450

4551
@pytest.mark.asyncio
4652
async def test_handle_429_rate_limit():
47-
"""Test handling of 429 rate limit errors."""
53+
"""429 errors must be retryable with a concrete wait window."""
4854
ctx = MagicMock()
4955
ctx.error = AsyncMock()
5056

51-
response = MagicMock()
52-
response.status_code = 429
53-
response.text = "Rate limit exceeded"
54-
55-
error = httpx.HTTPStatusError("", request=None, response=response)
56-
result = await handle_api_error(ctx, error, "api call")
57+
result = await handle_api_error(ctx, _make_http_error(429, "Rate limit exceeded"), "api call")
5758

5859
assert "Rate limit exceeded (429)" in result
5960
assert "try again later" in result
61+
# New: structured retryability marker
62+
assert "Retry: yes" in result
63+
assert "30" in result # mentions a concrete wait window
6064
ctx.error.assert_called_once()
6165

6266

6367
@pytest.mark.asyncio
6468
async def test_handle_500_server_error():
65-
"""Test handling of 500 server errors."""
69+
"""500 errors must be retryable and include the issues URL for persistent failures."""
6670
ctx = MagicMock()
6771
ctx.error = AsyncMock()
6872

69-
response = MagicMock()
70-
response.status_code = 500
71-
response.text = "Internal server error"
72-
73-
error = httpx.HTTPStatusError("", request=None, response=response)
74-
result = await handle_api_error(ctx, error, "operation")
73+
result = await handle_api_error(ctx, _make_http_error(500, "Internal server error"), "operation")
7574

7675
assert "Server error (500)" in result
7776
assert "CodeAlive service encountered an issue" in result
77+
assert "Retry: yes" in result
78+
assert "github.com/CodeAlive-AI/codealive-mcp/issues" in result
79+
80+
81+
@pytest.mark.asyncio
82+
async def test_handle_422_data_source_not_ready():
83+
"""422 errors must point at get_data_sources(alive_only=false)."""
84+
ctx = MagicMock()
85+
ctx.error = AsyncMock()
86+
87+
result = await handle_api_error(ctx, _make_http_error(422, "still indexing"), "search")
88+
89+
assert "(422)" in result
90+
assert "Retry: yes" in result
91+
assert "alive_only=false" in result
92+
93+
94+
@pytest.mark.asyncio
95+
async def test_handle_502_bad_gateway():
96+
ctx = MagicMock()
97+
ctx.error = AsyncMock()
98+
99+
result = await handle_api_error(ctx, _make_http_error(502), "search")
100+
101+
assert "(502)" in result
102+
assert "Retry: yes" in result
103+
assert "10" in result # wait window mentioned
104+
105+
106+
@pytest.mark.asyncio
107+
async def test_handle_503_service_unavailable():
108+
ctx = MagicMock()
109+
ctx.error = AsyncMock()
110+
111+
result = await handle_api_error(ctx, _make_http_error(503), "search")
112+
113+
assert "(503)" in result
114+
assert "Retry: yes" in result
78115

79116

80117
@pytest.mark.asyncio
81118
async def test_handle_generic_exception():
82-
"""Test handling of non-HTTP exceptions."""
119+
"""Non-HTTP exceptions still produce a structured Retry/Try message."""
83120
ctx = MagicMock()
84121
ctx.error = AsyncMock()
85122

@@ -88,27 +125,96 @@ async def test_handle_generic_exception():
88125

89126
assert "Error during parsing" in result
90127
assert "Invalid input" in result
91-
assert "check your input parameters" in result
128+
assert "Retry: no" in result
129+
assert "Try:" in result
92130

93131

94132
@pytest.mark.asyncio
95133
async def test_handle_unknown_http_error():
96-
"""Test handling of unknown HTTP status codes."""
134+
"""Unknown 4xx codes carry the raw detail (truncated) and a 'do not retry' marker."""
97135
ctx = MagicMock()
98136
ctx.error = AsyncMock()
99137

100-
response = MagicMock()
101-
response.status_code = 418 # I'm a teapot
102-
response.text = "I'm a teapot" * 100 # Long error text
103-
104-
error = httpx.HTTPStatusError("", request=None, response=response)
138+
error = _make_http_error(418, "I'm a teapot" * 100)
105139
result = await handle_api_error(ctx, error, "brewing")
106140

107141
assert "HTTP error: 418" in result
108-
# Should truncate long error messages
109-
assert len(result) < 300
142+
assert "Retry: no" in result
143+
# Detail is capped to 200 chars; the rest of the message adds ~120 chars of structured suffix
144+
assert len(result) < 400
145+
146+
147+
@pytest.mark.asyncio
148+
async def test_handle_timeout_error():
149+
"""Timeouts must be tagged retryable with explicit guidance."""
150+
ctx = MagicMock()
151+
ctx.error = AsyncMock()
152+
153+
result = await handle_api_error(ctx, httpx.ReadTimeout("slow"), "search")
154+
155+
assert "timeout" in result.lower()
156+
assert "Retry: yes" in result
157+
158+
159+
# ---------------------------------------------------------------------------
160+
# Per-tool recovery_hints overrides
161+
# ---------------------------------------------------------------------------
162+
163+
@pytest.mark.asyncio
164+
async def test_recovery_hints_override_default_404():
165+
"""A per-tool 404 hint must replace the generic one."""
166+
ctx = MagicMock()
167+
ctx.error = AsyncMock()
168+
169+
custom = "(1) check conversation_id, (2) drop conversation_id and retry"
170+
result = await handle_api_error(
171+
ctx, _make_http_error(404), "chat",
172+
recovery_hints={404: custom},
173+
)
174+
175+
assert "Not found error (404)" in result
176+
assert custom in result
177+
# Default 404 hint about get_data_sources is suppressed when overridden
178+
assert "get_data_sources" not in result
110179

111180

181+
@pytest.mark.asyncio
182+
async def test_recovery_hints_only_apply_to_matching_status():
183+
"""A 404 override must NOT change a 401 error."""
184+
ctx = MagicMock()
185+
ctx.error = AsyncMock()
186+
187+
result = await handle_api_error(
188+
ctx, _make_http_error(401), "chat",
189+
recovery_hints={404: "this should not appear"},
190+
)
191+
192+
assert "Authentication error (401)" in result
193+
assert "this should not appear" not in result
194+
# Default 401 hint is preserved
195+
assert "CODEALIVE_API_KEY" in result
196+
197+
198+
@pytest.mark.asyncio
199+
async def test_method_prefix_applied():
200+
"""The [method] prefix must be present in both returned and logged messages."""
201+
ctx = MagicMock()
202+
ctx.error = AsyncMock()
203+
204+
result = await handle_api_error(
205+
ctx, _make_http_error(401), "chat",
206+
method="codebase_consultant",
207+
)
208+
209+
assert result.startswith("[codebase_consultant] Error:")
210+
logged = ctx.error.call_args[0][0]
211+
assert logged.startswith("[codebase_consultant] ")
212+
213+
214+
# ---------------------------------------------------------------------------
215+
# format_data_source_names — unchanged behaviour
216+
# ---------------------------------------------------------------------------
217+
112218
def test_format_data_source_names_strings():
113219
"""Test formatting simple string names."""
114220
input_data = ["id1", "id2", "id3"]
@@ -152,4 +258,3 @@ def test_format_data_source_names_empty():
152258
assert format_data_source_names(None) == []
153259
assert format_data_source_names([]) == []
154260
assert format_data_source_names([None, "", {}]) == []
155-

src/tools/artifact_relationships.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,14 @@ async def get_artifact_relationships(
117117

118118
except (httpx.HTTPStatusError, Exception) as e:
119119
error_msg = await handle_api_error(
120-
ctx, e, "get artifact relationships", method=_TOOL_NAME
120+
ctx, e, "get artifact relationships", method=_TOOL_NAME,
121+
recovery_hints={
122+
404: (
123+
"(1) verify the identifier came from a recent codebase_search or fetch_artifacts result, "
124+
"(2) call codebase_search again to get a fresh identifier — the index may have changed, "
125+
"(3) check that the artifact is a function/class (relationships are not available for non-symbol artifacts)"
126+
),
127+
},
121128
)
122129
return json.dumps({"error": error_msg}, separators=(",", ":"))
123130

src/tools/chat.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,15 @@ async def codebase_consultant(
193193

194194
except (httpx.HTTPStatusError, Exception) as e:
195195
error_msg = await handle_api_error(
196-
ctx, e, "chat completion", method=_TOOL_NAME
196+
ctx, e, "chat completion", method=_TOOL_NAME,
197+
recovery_hints={
198+
404: (
199+
"(1) if continuing a conversation, verify conversation_id matches one returned by an earlier call, "
200+
"(2) if starting a new conversation, call get_data_sources to list valid data source names, "
201+
"(3) drop conversation_id and data_sources to fall back to the API key's default"
202+
),
203+
},
197204
)
198-
if isinstance(e, httpx.HTTPStatusError) and e.response.status_code == 404:
199-
return f"[{_TOOL_NAME}] Error: Not found (404): The requested resource could not be found. Check your conversation_id or data_sources."
200205
return error_msg
201206

202207

src/tools/datasources.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@ async def get_data_sources(ctx: Context, alive_only: bool = True) -> str:
129129

130130
except (httpx.HTTPStatusError, Exception) as e:
131131
error_msg = await handle_api_error(
132-
ctx, e, "retrieving data sources", method=_TOOL_NAME
132+
ctx, e, "retrieving data sources", method=_TOOL_NAME,
133+
recovery_hints={
134+
# 422 means *some* sources are still indexing — surface alive_only=false as the next step
135+
422: (
136+
"(1) call get_data_sources(alive_only=false) to see which sources are still being processed, "
137+
"(2) wait a few minutes for indexing to complete and retry"
138+
),
139+
},
133140
)
134141
return json.dumps({"error": error_msg}, separators=(",", ":"))

src/tools/fetch_artifacts.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,14 @@ async def fetch_artifacts(
108108

109109
except (httpx.HTTPStatusError, Exception) as e:
110110
error_msg = await handle_api_error(
111-
ctx, e, "fetch artifacts", method=_TOOL_NAME
111+
ctx, e, "fetch artifacts", method=_TOOL_NAME,
112+
recovery_hints={
113+
404: (
114+
"(1) verify the identifiers came from a recent codebase_search call (do not invent them), "
115+
"(2) re-run codebase_search to get fresh identifiers — the index may have changed, "
116+
"(3) for local repos in your working directory, use Read() on the file path instead"
117+
),
118+
},
112119
)
113120
return f"<error>{error_msg}</error>"
114121

src/tools/search.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,13 @@ async def codebase_search(
164164

165165
except (httpx.HTTPStatusError, Exception) as e:
166166
error_msg = await handle_api_error(
167-
ctx, e, "code search", method=_TOOL_NAME
167+
ctx, e, "code search", method=_TOOL_NAME,
168+
recovery_hints={
169+
404: (
170+
"(1) call get_data_sources to list available data source names, "
171+
"(2) check spelling and case of the names you passed in data_sources, "
172+
"(3) drop data_sources entirely to fall back to the API key's default"
173+
),
174+
},
168175
)
169-
if isinstance(e, httpx.HTTPStatusError) and e.response.status_code == 404:
170-
error_msg = f"[{_TOOL_NAME}] Error: Not found (404): One or more data sources could not be found. Check your data_sources."
171176
return json.dumps({"error": error_msg}, separators=(",", ":"))

0 commit comments

Comments
 (0)