-
Notifications
You must be signed in to change notification settings - Fork 130
feat: replace empty pass statements with error handling and logging #214
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
Open
Sainava
wants to merge
4
commits into
AOSSIE-Org:main
Choose a base branch
from
Sainava:feature/213-complete-incomplete-implementations
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
332fd0e
feat: replace empty pass statements with error handling and logging
Sainava 3b64e16
refactor: add exponential backoff with jitter to search retries
Sainava b204229
refactor: simplify Discord exception handling pattern
Sainava 4d99091
fix: remove f-string prefix without placeholders
Sainava File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,109 @@ | ||
| import asyncio | ||
| import logging | ||
| import random | ||
| from typing import List, Dict, Any | ||
| from ddgs import DDGS | ||
| from langsmith import traceable | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| class DuckDuckGoSearchTool: | ||
| """DDGS-based DuckDuckGo search integration""" | ||
| """DDGS-based DuckDuckGo search integration with configurable options. | ||
|
|
||
| Args: | ||
| timeout: Timeout for search requests in seconds (default: 10) | ||
| max_retries: Maximum number of retry attempts on failure (default: 2) | ||
| cache_enabled: Enable caching of search results (default: False) | ||
| base_delay: Base delay for exponential backoff in seconds (default: 1) | ||
| max_delay: Maximum delay between retries in seconds (default: 10) | ||
| """ | ||
|
|
||
| def __init__(self): | ||
| pass | ||
| def __init__(self, timeout: int = 10, max_retries: int = 2, cache_enabled: bool = False, | ||
| base_delay: float = 1.0, max_delay: float = 10.0): | ||
| self.timeout = timeout | ||
| self.max_retries = max_retries | ||
| self.cache_enabled = cache_enabled | ||
| self.base_delay = base_delay | ||
| self.max_delay = max_delay | ||
| self._cache: dict = {} if cache_enabled else None | ||
| logger.info(f"Initialized DuckDuckGoSearchTool (timeout={timeout}s, retries={max_retries}, cache={cache_enabled})") | ||
|
|
||
| def _perform_search(self, query: str, max_results: int): | ||
| with DDGS() as ddg: | ||
| return ddg.text(query, max_results=max_results) | ||
|
|
||
| @traceable(name="duckduckgo_search_tool", run_type="tool") | ||
| async def search(self, query: str, max_results: int = 5) -> List[Dict[str, Any]]: | ||
| try: | ||
| response = await asyncio.to_thread( | ||
| self._perform_search, | ||
| query=query, | ||
| max_results=max_results | ||
| ) | ||
| """Perform a DuckDuckGo search with caching and retry logic. | ||
|
|
||
| Args: | ||
| query: The search query string | ||
| max_results: Maximum number of results to return (default: 5) | ||
|
|
||
| Returns: | ||
| List of search results with title, content, URL, and score | ||
| """ | ||
| # Check cache if enabled | ||
| cache_key = f"{query}:{max_results}" | ||
| if self.cache_enabled and cache_key in self._cache: | ||
| logger.debug(f"Returning cached results for query: {query}") | ||
| return self._cache[cache_key] | ||
|
|
||
| # Retry logic | ||
| last_exception = None | ||
| for attempt in range(self.max_retries + 1): | ||
| try: | ||
| logger.debug(f"Search attempt {attempt + 1}/{self.max_retries + 1} for query: {query}") | ||
| response = await asyncio.wait_for( | ||
| asyncio.to_thread( | ||
| self._perform_search, | ||
| query=query, | ||
| max_results=max_results | ||
| ), | ||
| timeout=self.timeout | ||
| ) | ||
|
|
||
| results = [] | ||
| for result in response or []: | ||
| results.append({ | ||
| "title": result.get("title", ""), | ||
| "content": result.get("body", ""), | ||
| "url": result.get("href", ""), | ||
| "score": 0 | ||
| }) | ||
|
|
||
| # Cache results if enabled | ||
| if self.cache_enabled: | ||
| self._cache[cache_key] = results | ||
| logger.debug(f"Cached {len(results)} results for query: {query}") | ||
|
|
||
| logger.info(f"Successfully retrieved {len(results)} results for query: {query}") | ||
| return results | ||
|
|
||
| except (asyncio.TimeoutError, TimeoutError): | ||
| last_exception = TimeoutError(f"Search timed out after {self.timeout}s") | ||
| logger.warning(f"Search timeout (attempt {attempt + 1}/{self.max_retries + 1}): {query}") | ||
| if attempt < self.max_retries: | ||
| delay = min(self.base_delay * (2 ** attempt) + random.uniform(-0.1, 0.1), self.max_delay) | ||
| await asyncio.sleep(delay) | ||
| continue | ||
|
|
||
| except ConnectionError as e: | ||
| last_exception = e | ||
| logger.warning(f"Network issue (attempt {attempt + 1}/{self.max_retries + 1}): {e}") | ||
| if attempt < self.max_retries: | ||
| delay = min(self.base_delay * (2 ** attempt) + random.uniform(-0.1, 0.1), self.max_delay) | ||
| await asyncio.sleep(delay) | ||
| continue | ||
|
|
||
| results = [] | ||
| for result in response or []: | ||
| results.append({ | ||
| "title": result.get("title", ""), | ||
| "content": result.get("body", ""), | ||
| "url": result.get("href", ""), | ||
| "score": 0 | ||
| }) | ||
| return results | ||
| except Exception as e: | ||
| last_exception = e | ||
| logger.error(f"Search error (attempt {attempt + 1}/{self.max_retries + 1}): {str(e)}") | ||
| if attempt < self.max_retries: | ||
| delay = min(self.base_delay * (2 ** attempt) + random.uniform(-0.1, 0.1), self.max_delay) | ||
| await asyncio.sleep(delay) | ||
| continue | ||
|
|
||
| except (ConnectionError, TimeoutError) as e: | ||
| logger.warning("Network issue during DDG search: %s", e) | ||
| return [] | ||
| except Exception as e: | ||
| logger.error("DuckDuckGo search failed: %s", str(e)) | ||
| return [] | ||
| # All retries failed | ||
| logger.error(f"All {self.max_retries + 1} search attempts failed for query: {query}. Last error: {last_exception}") | ||
| return [] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import logging | ||
| import discord | ||
|
|
||
| from app.agents.devrel.onboarding.messages import ( | ||
|
|
@@ -7,6 +8,8 @@ | |
| ) | ||
| from app.services.auth.management import get_or_create_user_by_discord | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def build_final_handoff_embed() -> discord.Embed: | ||
| """Create the final hand-off embed describing capabilities.""" | ||
|
|
@@ -29,9 +32,13 @@ async def send_final_handoff_dm(user: discord.abc.User): | |
| try: | ||
| embed = build_final_handoff_embed() | ||
| await user.send(embed=embed) | ||
| except Exception: | ||
| # Fail silently to avoid crashing flows if DMs are closed or similar | ||
| pass | ||
| logger.info(f"Successfully sent handoff DM to user {user.id}") | ||
| except discord.Forbidden: | ||
| logger.warning(f"Cannot send DM to user {user.id} - DMs are disabled or bot is blocked") | ||
| except discord.HTTPException as e: | ||
| logger.error(f"Discord API error sending DM to user {user.id}: {e.status} - {e.text}") | ||
| except Exception as e: | ||
| logger.error(f"Unexpected error sending DM to user {user.id}: {type(e).__name__} - {str(e)}") | ||
|
|
||
| class OAuthView(discord.ui.View): | ||
| """View with OAuth button.""" | ||
|
|
@@ -99,8 +106,12 @@ async def check_verified( # type: ignore[override] | |
| await send_final_handoff_dm(interaction.user) | ||
| try: | ||
| await interaction.message.edit(view=self) | ||
| except Exception: | ||
| pass | ||
| except discord.NotFound: | ||
| logger.warning(f"Message not found when editing onboarding view for user {interaction.user.id}") | ||
| except discord.HTTPException as e: | ||
| logger.error(f"Failed to edit onboarding view: {e.status} - {e.text}") | ||
| except Exception as e: | ||
| logger.error(f"Unexpected error editing onboarding view: {type(e).__name__} - {str(e)}") | ||
|
||
| else: | ||
| await interaction.followup.send( | ||
| "I still don't see a linked GitHub account. Run `/verify_github` and try again in a moment.", | ||
|
|
@@ -115,6 +126,9 @@ async def skip(self, interaction: discord.Interaction, button: discord.ui.Button | |
| item.disabled = True | ||
| try: | ||
| await interaction.response.edit_message(view=self) | ||
| except Exception: | ||
| # If edit fails (e.g., message deleted), ignore | ||
| pass | ||
| except discord.NotFound: | ||
| logger.warning(f"Message not found when editing skip view for user {interaction.user.id}") | ||
| except discord.HTTPException as e: | ||
| logger.error(f"Failed to edit skip view: {e.status} - {e.text}") | ||
| except Exception as e: | ||
| logger.error(f"Unexpected error editing skip view: {type(e).__name__} - {str(e)}") | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.