🧹 Refactor: Remove bare except Exception: suppressing errors silently#227
Conversation
Modifies `_resolve_indexer_from_container` in `src/codeweaver/server/agent_api/search/__init__.py` to capture exceptions as `e` and log them using `logger.warning`. This improves observability and code health by preventing silent failures without changing the return logic. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR refactors the error handling in Sequence diagram for IndexingService resolution with loggingsequenceDiagram
participant Caller
participant _resolve_indexer_from_container
participant Container as DIContainer
participant IndexingService
participant Logger
Caller->>_resolve_indexer_from_container: call
activate _resolve_indexer_from_container
_resolve_indexer_from_container->>Container: get_container()
activate Container
Container-->>_resolve_indexer_from_container: container
deactivate Container
_resolve_indexer_from_container->>Container: resolve(IndexingService)
alt resolution_succeeds
Container-->>_resolve_indexer_from_container: IndexingService instance
_resolve_indexer_from_container-->>Caller: IndexingService instance
else resolution_raises_exception
Container--x _resolve_indexer_from_container: Exception e
_resolve_indexer_from_container->>Logger: warning(Failed to resolve IndexingService from container: e)
_resolve_indexer_from_container-->>Caller: None
end
deactivate _resolve_indexer_from_container
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider including
exc_info=Truein the warning log so that the full traceback is captured for easier debugging whenIndexingServiceresolution fails.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider including `exc_info=True` in the warning log so that the full traceback is captured for easier debugging when `IndexingService` resolution fails.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Refactors _resolve_indexer_from_container to avoid silently swallowing DI container resolution errors by logging failures while preserving the None fallback.
Changes:
- Captures the thrown exception during container resolution and logs a warning.
- Keeps the existing fallback behavior by returning
Nonewhen resolution fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors DI container resolution to avoid silently swallowing exceptions by logging warning details while preserving the existing None fallback behavior.
Changes:
- Log warnings (with stack traces) when resolving
IndexingServicefrom the DI container fails. - Include exception tracebacks in the existing “Auto-indexing failed” warning.
Comments suppressed due to low confidence (1)
src/codeweaver/server/agent_api/search/init.py:1
- Catching
Exceptionin anasync defcan inadvertently swallow task cancellation on Python versions whereasyncio.CancelledErrorsubclassesException, preventing cooperative cancellation. Prefer handling cancellation explicitly (e.g.,except asyncio.CancelledError: raise) before the broadexcept Exceptionblock.
# SPDX-FileCopyrightText: 2026 Knitli Inc.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # We enable reconciliation by default to fix any partial indexes | ||
| await indexer.index_project(add_dense=True, add_sparse=True) | ||
| except Exception as e: | ||
| logger.warning("Auto-indexing failed: %s", e) | ||
| logger.warning("Auto-indexing failed: %s", e, exc_info=True) |
| "Failed to resolve IndexingService from container: %s", | ||
| e, | ||
| exc_info=True, | ||
| ) |
…ly (#227) * Refactor: Remove bare `except Exception:` suppressing errors silently Modifies `_resolve_indexer_from_container` in `src/codeweaver/server/agent_api/search/__init__.py` to capture exceptions as `e` and log them using `logger.warning`. This improves observability and code health by preventing silent failures without changing the return logic. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com> --------- Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
🎯 What: The code health issue addressed was a bare
except Exception:clause in_resolve_indexer_from_containerthat silently swallowed any exceptions thrown during DI container resolution, returningNonewithout leaving a trace.💡 Why: This improves maintainability by ensuring that any actual errors (e.g. issues during container resolution) are properly logged as warnings instead of failing silently. This provides essential context for debugging should the
IndexingServicefail to resolve.✅ Verification: Verified by inspecting the file to confirm
loggeris imported and set up. Executeduv run pytest tests/unit/server/agent_api/find_code/ --no-covensuring all 9 unit tests still pass successfully with no regressions.✨ Result: The system will now emit a warning log with the actual exception details when container resolution fails, while correctly preserving the original fallback logic (returning
None).PR created automatically by Jules for task 4193187016987027480 started by @bashandbone
Summary by Sourcery
Enhancements: