-
Notifications
You must be signed in to change notification settings - Fork 112
Differentiate retryable and non-retryable indexing errors #220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,7 +114,10 @@ async def index_repo(self, repo_input: str, discord_id: str) -> Dict[str, Any]: | |
| } | ||
| ) as response: | ||
| if response.status == 200: | ||
| data = await response.json() if await response.text() else {} | ||
| try: | ||
| data = await response.json() | ||
| except (aiohttp.ContentTypeError, ValueError): | ||
| data = {} | ||
|
|
||
| await self.supabase.table("indexed_repositories").update({ | ||
| "indexing_status": "completed", | ||
|
|
@@ -133,8 +136,9 @@ async def index_repo(self, repo_input: str, discord_id: str) -> Dict[str, Any]: | |
| "nodes": data.get("node_count", 0), | ||
| "edges": data.get("edge_count", 0) | ||
| } | ||
| else: | ||
| error_msg = (await response.text())[:500] | ||
| elif response.status == 429: | ||
| error_text = await response.text() | ||
| error_msg = error_text[:500] | ||
|
|
||
| await self.supabase.table("indexed_repositories").update({ | ||
| "indexing_status": "failed", | ||
|
|
@@ -143,16 +147,69 @@ async def index_repo(self, repo_input: str, discord_id: str) -> Dict[str, Any]: | |
| "is_deleted", False | ||
| ).execute() | ||
|
|
||
| return {"status": "error", "message": f"Indexing failed: {error_msg}"} | ||
| return { | ||
| "status": "error", | ||
| "error_code": "RATE_LIMITED", | ||
| "retryable": True, | ||
| "message": "Rate limit exceeded. Please try again later." | ||
| } | ||
| elif 500 <= response.status < 600: | ||
| error_text = await response.text() | ||
| error_msg = error_text[:500] | ||
|
|
||
| await self.supabase.table("indexed_repositories").update({ | ||
| "indexing_status": "failed", | ||
| "last_error": error_msg | ||
| }).eq("repository_full_name", repo_info['full_name']).eq( | ||
| "is_deleted", False | ||
| ).execute() | ||
|
|
||
| return { | ||
| "status": "error", | ||
| "error_code": "BACKEND_ERROR", | ||
| "retryable": True, | ||
| "message": "Indexing service encountered an internal error. Please try again later." | ||
| } | ||
| else: | ||
| error_text = await response.text() | ||
| error_msg = error_text[:500] | ||
|
|
||
| await self.supabase.table("indexed_repositories").update({ | ||
| "indexing_status": "failed", | ||
| "last_error": error_msg | ||
| }).eq("repository_full_name", repo_info['full_name']).eq( | ||
| "is_deleted", False | ||
| ).execute() | ||
|
|
||
| return { | ||
| "status": "error", | ||
| "error_code": "INVALID_REQUEST", | ||
| "retryable": False, | ||
| "message": "Indexing request was rejected. Please verify the repository." | ||
| } | ||
| except ValueError as e: | ||
| return {"status": "error", "message": str(e)} | ||
| return { | ||
| "status": "error", | ||
| "error_code": "INVALID_REPOSITORY", | ||
| "retryable": False, | ||
| "message": str(e) | ||
| } | ||
| except aiohttp.ClientError as e: | ||
| logger.exception(f"Network error indexing {repo_input}: {e}") | ||
| return {"status": "error", "message": "Network error. Please try again."} | ||
| return { | ||
| "status": "error", | ||
| "error_code": "BACKEND_UNAVAILABLE", | ||
| "retryable": True, | ||
| "message": "Indexing service is currently unavailable. Please try again later." | ||
| } | ||
|
Comment on lines
+199
to
+204
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Database status not updated on ClientError. Similar to the ValueError case, if a database record was created or updated to "pending" status (lines 80-94) before the ClientError occurs, it will remain in "pending" state permanently, blocking retry attempts. π Proposed fix: Update database before returning except aiohttp.ClientError as e:
logger.exception(f"Network error indexing {repo_input}: {e}")
+
+ # Update database if record exists
+ try:
+ error_msg = f"Network error: {str(e)}"[:500]
+ await self.supabase.table("indexed_repositories").update({
+ "indexing_status": "failed",
+ "last_error": error_msg
+ }).eq("repository_full_name", repo_info.get('full_name', repo_input)).eq(
+ "is_deleted", False
+ ).execute()
+ except Exception:
+ logger.exception("Failed to update database after ClientError")
+
return {
"status": "error",
"error_code": "BACKEND_UNAVAILABLE",
"retryable": True,
"message": "Indexing service is currently unavailable. Please try again later."
}
|
||
| except Exception: | ||
| logger.exception(f"Failed to index {repo_input}") | ||
| return {"status": "error", "message": "Indexing failed. Please contact support."} | ||
| return { | ||
| "status": "error", | ||
| "error_code": "INDEXING_FAILED", | ||
| "retryable": False, | ||
| "message": "Indexing failed due to an unexpected error." | ||
| } | ||
|
Comment on lines
+207
to
+212
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Database status not updated on general Exception. If a database record was created or updated to "pending" status (lines 80-94) before an unexpected exception occurs, it will remain in "pending" state permanently, blocking retry attempts. π Proposed fix: Update database before returning except Exception:
logger.exception(f"Failed to index {repo_input}")
+
+ # Update database if record exists
+ try:
+ await self.supabase.table("indexed_repositories").update({
+ "indexing_status": "failed",
+ "last_error": "Indexing failed due to an unexpected error."
+ }).eq("repository_full_name", repo_info.get('full_name', repo_input)).eq(
+ "is_deleted", False
+ ).execute()
+ except Exception:
+ logger.exception("Failed to update database after unexpected exception")
+
return {
"status": "error",
"error_code": "INDEXING_FAILED",
"retryable": False,
"message": "Indexing failed due to an unexpected error."
}
|
||
|
|
||
| async def query_repo(self, question: str, repo_full_name: str) -> Dict[str, Any]: | ||
| """ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Database status not updated on ValueError.
If a database record was created or updated to "pending" status (lines 80-94) before the ValueError occurs, it will remain in "pending" state permanently. This breaks retry functionality because lines 71-75 will block subsequent attempts with "indexing in progress."
π Proposed fix: Update database before returning
Note:
repo_infomight not be available if parsing failed, so userepo_inputas fallback or extract owner/repo before the try-except if possible.π€ Prompt for AI Agents