Skip to content

Commit c1eb7a3

Browse files
committed
fix: address review findings -- 5 code fixes, 2 test improvements
Code fixes: 1. repos.py add_repo: status 400 -> 500 (server failure, not client error) 2. repos.py _run_async_indexing: publisher.publish_error leaks str(e) to WebSocket clients. Now sends generic message. 3. repos.py websocket_index: send_json leaks str(e). Now generic. 4. playground.py _fetch_repo_metadata: returned raw str(e) in error dict. Now returns generic message, logs actual error. 5. playground.py search handler: added capture_exception (was missing entirely, not even imported). 6. analysis.py impact: added file_path to capture_exception kwargs (logger had it, Sentry didn't). Test improvements: 7. test_repo_add: assert status 500 (was 400), assert exact detail text. 8. test_search: explicitly patch cache.get_search_results to return None instead of relying on conftest mock behavior. 284 tests pass.
1 parent 66792cb commit c1eb7a3

4 files changed

Lines changed: 14 additions & 11 deletions

File tree

backend/routes/analysis.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ async def analyze_impact(
7878
raise
7979
except Exception as e:
8080
logger.error("Impact analysis failed", repo_id=repo_id, file_path=request.file_path, error=str(e))
81-
capture_exception(e, operation="impact_analysis", repo_id=repo_id)
81+
capture_exception(e, operation="impact_analysis", repo_id=repo_id, file_path=request.file_path)
8282
raise HTTPException(status_code=500, detail="Failed to analyze impact")
8383

8484

backend/routes/playground.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from dependencies import indexer, cache, repo_manager, redis_client
1818
from services.input_validator import InputValidator
1919
from services.repo_validator import RepoValidator
20-
from services.observability import logger
20+
from services.observability import logger, capture_exception
2121
from services.playground_limiter import PlaygroundLimiter, get_playground_limiter, IndexedRepoData
2222
from services.anonymous_indexer import (
2323
AnonymousIndexingJob,
@@ -486,6 +486,7 @@ async def playground_search(
486486
except HTTPException:
487487
raise
488488
except Exception as e:
489+
capture_exception(e, operation="playground_search")
489490
logger.error("Playground search failed", error=str(e))
490491
raise HTTPException(status_code=500, detail="Search failed")
491492

@@ -579,7 +580,7 @@ async def _fetch_repo_metadata(owner: str, repo: str) -> dict:
579580
return {"error": "timeout", "message": "GitHub API request timed out"}
580581
except Exception as e:
581582
logger.error("GitHub API request failed", error=str(e))
582-
return {"error": "request_failed", "message": str(e)}
583+
return {"error": "request_failed", "message": "Failed to fetch repository metadata"}
583584

584585

585586
async def _count_code_files(

backend/routes/repos.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ async def add_repository(
145145
except Exception as e:
146146
logger.error("Failed to add repository", error=str(e), user_id=user_id)
147147
capture_exception(e)
148-
raise HTTPException(status_code=400, detail="Failed to add repository")
148+
raise HTTPException(status_code=500, detail="Failed to add repository")
149149

150150

151151
@router.delete("/{repo_id}")
@@ -395,7 +395,7 @@ async def progress_callback(
395395
publisher.publish_error(
396396
repo_id,
397397
error="indexing_failed",
398-
message=str(e),
398+
message="An error occurred during indexing",
399399
recoverable=True
400400
)
401401

@@ -573,7 +573,7 @@ async def progress_callback(files_processed: int, functions_indexed: int, total_
573573
logger.error("WebSocket indexing error", repo_id=repo_id, error=str(e))
574574
capture_exception(e, operation="websocket_indexing", repo_id=repo_id)
575575
try:
576-
await websocket.send_json({"type": "error", "message": str(e)})
576+
await websocket.send_json({"type": "error", "message": "An error occurred during indexing"})
577577
except Exception:
578578
pass
579579
repo_manager.update_status(repo_id, "error")

backend/tests/test_error_leaking.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ class TestErrorResponsesHideInternals:
88

99
def test_search_error_hides_details(self, client, valid_headers):
1010
"""Search failure should not expose Pinecone/OpenAI error strings."""
11-
with patch("routes.search.indexer") as mock_indexer:
11+
with patch("routes.search.indexer") as mock_indexer, \
12+
patch("routes.search.cache") as mock_cache:
13+
mock_cache.get_search_results.return_value = None
1214
mock_indexer.semantic_search.side_effect = RuntimeError(
1315
"Pinecone connection refused at pinecone-prod.svc.us-east1.aws:443"
1416
)
@@ -21,9 +23,9 @@ def test_search_error_hides_details(self, client, valid_headers):
2123

2224
assert resp.status_code == 500
2325
body = resp.json()["detail"]
26+
assert body == "Search failed"
2427
assert "Pinecone" not in body
2528
assert "pinecone-prod" not in body
26-
assert "443" not in body
2729

2830
def test_dependency_graph_error_hides_details(self, client, valid_headers):
2931
"""Dependency graph failure should not expose file paths."""
@@ -41,8 +43,8 @@ def test_dependency_graph_error_hides_details(self, client, valid_headers):
4143

4244
assert resp.status_code == 500
4345
body = resp.json()["detail"]
46+
assert body == "Failed to build dependency graph"
4447
assert "/srv/repos" not in body
45-
assert ".git/config" not in body
4648

4749
def test_repo_add_error_hides_details(self, client, valid_headers):
4850
"""Add repo failure should not expose git credentials or paths."""
@@ -60,7 +62,7 @@ def test_repo_add_error_hides_details(self, client, valid_headers):
6062
headers=valid_headers,
6163
)
6264

63-
assert resp.status_code == 400
65+
assert resp.status_code == 500
6466
body = resp.json()["detail"]
67+
assert body == "Failed to add repository"
6568
assert "ghp_secret" not in body
66-
assert "Authentication failed" not in body

0 commit comments

Comments
 (0)