fix(tasks): narrow broad exception handlers to specific types#2339
fix(tasks): narrow broad exception handlers to specific types#2339Br1an67 wants to merge 1 commit intotopoteretes:mainfrom
Conversation
WalkthroughThe PR narrows overly broad exception handlers across three files in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cognee/tasks/chunks/create_chunk_associations.py`:
- Around line 218-220: The except block catching (OSError, ValueError,
RuntimeError) after graph_engine.add_edges() misses backend-specific errors
(e.g., neo4j.exceptions.Neo4jError) and other drivers; update the handler to
either explicitly catch known backend exception types (for example Neo4jError)
in addition to RuntimeError or broaden to catch Exception so all graph-engine
errors are handled uniformly, and adjust the index_graph_edges() docstring to
state which exceptions may be raised/normalized; locate the handling around
graph_engine.add_edges() and the index_graph_edges() docstring to make the
changes.
In `@cognee/tasks/codingagents/coding_rule_associations.py`:
- Around line 85-86: The current except TypeError handler around code that uses
getattr(rule, "id", None) and builds tuples/dicts is misleading; inspect the try
block in coding_rule_associations (the code that references rule and builds the
association tuples/dicts) and either (A) validate the rule shape before the try
(e.g., check hasattr(rule, "id") or isinstance(rule, expected_type)) and remove
the catch, or (B) if you need defensive catching, change except TypeError to a
more appropriate exception (e.g., except AttributeError or except Exception) and
add a clarifying comment above the handler explaining which malformed rule
conditions are expected; also keep the logger.info call but include enough
context (rule repr or id) so the log is useful.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 449c49e4-39b4-49ef-89d0-bec84580f252
📒 Files selected for processing (3)
cognee/shared/logging_utils.pycognee/tasks/chunks/create_chunk_associations.pycognee/tasks/codingagents/coding_rule_associations.py
| except (OSError, ValueError, RuntimeError) as e: | ||
| logger.error(f"Failed to persist edges to graph database: {e}") | ||
| raise |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check graph engine add_edges implementations
rg -n "async def add_edges" cognee/infrastructure/databases/graph/ --type=py -A10
# Check what exceptions graph operations can raise
rg -n "raise\s+" cognee/infrastructure/databases/graph/ --type=py -C2Repository: topoteretes/cognee
Length of output: 43333
🏁 Script executed:
# Check the actual file and surrounding context
cat -n cognee/tasks/chunks/create_chunk_associations.py | sed -n '200,230p'
# Search for index_graph_edges function definition
rg -n "def index_graph_edges" cognee/ --type=py -A5Repository: topoteretes/cognee
Length of output: 1664
🏁 Script executed:
# Get the full index_graph_edges function
cat -n cognee/tasks/storage/index_graph_edges.py | head -100
# Check for exception raises in that file
rg -n "raise" cognee/tasks/storage/index_graph_edges.py -B2 -A2Repository: topoteretes/cognee
Length of output: 3491
🏁 Script executed:
# Check Kuzu adapter's add_edges method implementation
sed -n '738,800p' cognee/infrastructure/databases/graph/kuzu/adapter.py
# Check what exceptions the method can raise
rg -n "async def add_edges" cognee/infrastructure/databases/graph/kuzu/adapter.py -A60 | head -80Repository: topoteretes/cognee
Length of output: 5097
🏁 Script executed:
# Check Neo4jError and its inheritance
rg -n "class Neo4jError" cognee/ --type=py
# Check Neo4j imports and error types
rg -n "from neo4j" cognee/infrastructure/databases/graph/neo4j_driver/ --type=py | head -20
# Verify what the get_graph_engine can raise
sed -n '100,160p' cognee/infrastructure/databases/graph/get_graph_engine.pyRepository: topoteretes/cognee
Length of output: 2764
🏁 Script executed:
# Check Neo4jError inheritance - is it a subclass of any of OSError, ValueError, or RuntimeError?
python3 << 'EOF'
try:
from neo4j.exceptions import Neo4jError
# Check the MRO (Method Resolution Order)
print("Neo4jError MRO:", Neo4jError.__mro__)
print("\nIs Neo4jError a subclass of OSError?", issubclass(Neo4jError, OSError))
print("Is Neo4jError a subclass of ValueError?", issubclass(Neo4jError, ValueError))
print("Is Neo4jError a subclass of RuntimeError?", issubclass(Neo4jError, RuntimeError))
print("Is Neo4jError a subclass of Exception?", issubclass(Neo4jError, Exception))
except ImportError:
print("neo4j not available in sandbox")
EOF
# Check Neptune exceptions
rg -n "class.*Error\|raise.*Error" cognee/infrastructure/databases/graph/neptune_driver/adapter.py | head -30Repository: topoteretes/cognee
Length of output: 92
🏁 Script executed:
# Check the docstring for create_chunk_associations.py to see what exceptions it should handle
sed -n '1,100p' cognee/tasks/chunks/create_chunk_associations.py | grep -A50 "def create_chunk_associations"
# Check if there's any Exception parent class handling elsewhere
rg -n "except Exception" cognee/tasks/chunks/create_chunk_associations.pyRepository: topoteretes/cognee
Length of output: 172
🏁 Script executed:
# Check the docstring around lines 132-134 mentioned in the review
sed -n '120,145p' cognee/tasks/chunks/create_chunk_associations.py
# Check if there's a function docstring for create_chunk_associations
sed -n '1,200p' cognee/tasks/chunks/create_chunk_associations.py | grep -B5 -A30 "Raises:"Repository: topoteretes/cognee
Length of output: 2211
🏁 Script executed:
# Check NeptuneAnalyticsConfigurationError definition
rg -n "class NeptuneAnalyticsConfigurationError" cognee/ --type=py
# Get full context of the try-except block in create_chunk_associations
sed -n '210,225p' cognee/tasks/chunks/create_chunk_associations.py
# Verify what exceptions get_graph_engine can raise during operation
sed -n '80,200p' cognee/infrastructure/databases/graph/get_graph_engine.py | grep -A3 "raise"Repository: topoteretes/cognee
Length of output: 2282
🏁 Script executed:
# Check what exceptions can propagate from index_data_points
rg -n "async def index_data_points" cognee/ --type=py -A30
# Check if there's a test for this exception handling
fd -e py -x grep -l "create_chunk_associations\|index_graph_edges" cognee/tests/ 2>/dev/null | head -5Repository: topoteretes/cognee
Length of output: 20110
🏁 Script executed:
# Check the test for chunk associations
cat -n cognee/tests/test_chunk_associations.pyRepository: topoteretes/cognee
Length of output: 10077
Catch backend-specific graph database exceptions or use a broader exception type.
The graph_engine.add_edges() call will raise Neo4jError (from neo4j.exceptions) when using the Neo4j backend, which is not caught by the current (OSError, ValueError, RuntimeError) tuple. Neptune and other backends may raise additional exception types not covered here.
Since index_graph_edges() already wraps initialization errors as RuntimeError (line 70-72), consider either catching the backend-specific exceptions explicitly or using a broader Exception catch if all exceptions should be treated identically. Update the docstring accordingly to reflect what can actually be raised.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cognee/tasks/chunks/create_chunk_associations.py` around lines 218 - 220, The
except block catching (OSError, ValueError, RuntimeError) after
graph_engine.add_edges() misses backend-specific errors (e.g.,
neo4j.exceptions.Neo4jError) and other drivers; update the handler to either
explicitly catch known backend exception types (for example Neo4jError) in
addition to RuntimeError or broaden to catch Exception so all graph-engine
errors are handled uniformly, and adjust the index_graph_edges() docstring to
state which exceptions may be raised/normalized; locate the handling around
graph_engine.add_edges() and the index_graph_edges() docstring to make the
changes.
| except TypeError as e: | ||
| logger.info(f"Warning: Skipping invalid rule due to error: {e}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what types Rule objects are and how they're instantiated
ast-grep --pattern 'class Rule($_) {
$$$
}'
# Search for any existing exception handling patterns for similar rule processing
rg -n "except.*Error.*rule" --type=py -C2Repository: topoteretes/cognee
Length of output: 44
🏁 Script executed:
cat -n cognee/tasks/codingagents/coding_rule_associations.py | head -100Repository: topoteretes/cognee
Length of output: 4111
🏁 Script executed:
cd cognee && git log --oneline -n 20 -- tasks/codingagents/coding_rule_associations.py 2>/dev/null || echo "Not a git repo or file not in history"Repository: topoteretes/cognee
Length of output: 132
🏁 Script executed:
cd cognee && git show HEAD:tasks/codingagents/coding_rule_associations.py 2>/dev/null | sed -n '68,90p' || rg -A 15 "for rule in rules:" cognee/tasks/codingagents/coding_rule_associations.pyRepository: topoteretes/cognee
Length of output: 44
🏁 Script executed:
cd cognee && git show 156a0809:tasks/codingagents/coding_rule_associations.py 2>/dev/null | sed -n '68,90p'Repository: topoteretes/cognee
Length of output: 44
🏁 Script executed:
git -C cognee show 156a0809:tasks/codingagents/coding_rule_associations.py 2>/dev/null | head -100Repository: topoteretes/cognee
Length of output: 44
🏁 Script executed:
git -C cognee log -p --max-count=1 156a0809 -- tasks/codingagents/coding_rule_associations.py 2>/dev/null | head -150Repository: topoteretes/cognee
Length of output: 5737
Clarify the exception type being caught in this defensive handler.
The operations in the try block do not appear to raise TypeError: getattr(rule, "id", None) returns None rather than raising an exception, and tuple/dict construction in Python accepts any types. If the goal is to handle malformed rule objects gracefully, consider verifying what exception TypeError is actually intended to catch here, or if a broader exception type (or different exception) would be more appropriate. Add a comment explaining the catch if the intent is non-obvious.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cognee/tasks/codingagents/coding_rule_associations.py` around lines 85 - 86,
The current except TypeError handler around code that uses getattr(rule, "id",
None) and builds tuples/dicts is misleading; inspect the try block in
coding_rule_associations (the code that references rule and builds the
association tuples/dicts) and either (A) validate the rule shape before the try
(e.g., check hasattr(rule, "id") or isinstance(rule, expected_type)) and remove
the catch, or (B) if you need defensive catching, change except TypeError to a
more appropriate exception (e.g., except AttributeError or except Exception) and
add a clarifying comment above the handler explaining which malformed rule
conditions are expected; also keep the logger.info call but include enough
context (rule repr or id) so the log is useful.
Narrowed broad exception handlers in priority files to catch specific exception types (OSError, ValueError, RuntimeError) instead of generic Exception. This aligns with PEP8 recommendations for exception handling. - cognee/tasks/chunks/create_chunk_associations.py: Narrowed LLM comparison and vector search exception handlers - cognee/tasks/codingagents/coding_rule_associations.py: Narrowed exception handler to TypeError - cognee/shared/logging_utils.py: Replaced bare except clauses with OSError These changes allow unexpected bugs to bubble up properly while maintaining graceful error handling for expected I/O errors, query errors, and runtime errors. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: root <root@C20251020184286.local> Signed-off-by: Br1an67 <932039080@qq.com>
Br1an67
force-pushed
the
fix/issue-2318-narrow-exception-handlers
branch
from
156a080 to
2dc446f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
cognee/tasks/codingagents/coding_rule_associations.py (1)
68-86:⚠️ Potential issue | 🟡 MinorRemove the dead
TypeErrorhandler.This block does not look capable of raising
TypeErrorfor a malformed rule in the common case:getattr(rule, "id", None)already falls back toNone, and the rest is just tuple/dict construction pluslist.append(). Prefer an explicitrule_id is Noneguard here and let unexpected getter bugs propagate. Based on learnings: Applies to **/*.py : Prefer explicit, structured error handling in Python code.#!/bin/bash # Verify whether the guarded operations can realistically raise TypeError # and inspect how `id` is defined on DataPoint/related models. sed -n '66,86p' cognee/tasks/codingagents/coding_rule_associations.py rg -n -C3 'class DataPoint\b|class Rule\b|^[[:space:]]*id[[:space:]]*:' cognee --type=py rg -n -C3 '\bgetattr\s*\(\s*rule\s*,\s*"id"\s*,\s*None\s*\)' cognee/tasks/codingagents/coding_rule_associations.pyIf
idis just a normal model field, this catch should be removed and replaced with an explicit skip onNone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cognee/tasks/codingagents/coding_rule_associations.py` around lines 68 - 86, Remove the dead TypeError handler around the rule processing: drop the try/except and instead explicitly check whether rule_id = getattr(rule, "id", None) is None and skip appending to relationships when it is, logging the skip via logger.info if desired; leave all other operations (tuple/dict construction and relationships.append) unguarded so real errors from rule access surface normally, referencing the variables rule, rule_id, origin_id, relationships, and logger in your change.cognee/tasks/chunks/create_chunk_associations.py (1)
131-134:⚠️ Potential issue | 🟠 MajorCatch the graph driver's exception base here as well.
graph_engine.add_edges()can fail with backend-specific database exceptions that are not covered by(OSError, ValueError, RuntimeError). When that happens, this log/re-raise path is skipped, and the updatedRaises:section also understates what can escape from this boundary. Either include the driver exception base(s) raised byadd_edges()here, or keep a broader catch at this boundary if all persistence failures should be logged and re-raised uniformly.#!/bin/bash # Inspect graph backends for add_edges implementations and the exception families they use. rg -n -C3 'async def add_edges\(' cognee/infrastructure/databases/graph --type=py rg -n -C2 'Neo4jError|from neo4j\.exceptions|raise .*Error' \ cognee/infrastructure/databases/graph --type=py sed -n '131,140p' cognee/tasks/chunks/create_chunk_associations.py sed -n '212,220p' cognee/tasks/chunks/create_chunk_associations.pyIf
add_edges()can raise driver-specific exceptions, extend this catch/docstring to match the actual persistence error surface.Also applies to: 213-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cognee/tasks/chunks/create_chunk_associations.py` around lines 131 - 134, The except block that catches (OSError, ValueError, RuntimeError) around graph_engine.add_edges() in create_chunk_associations.py misses backend-specific driver exceptions and the docstring's Raises list is incomplete; update the try/except to also catch the graph driver's base exception(s) (or use a broader persistence failure base like Exception if appropriate for uniform logging/re-raising) so all persistence errors from graph_engine.add_edges() are logged and re-raised, and update the Raises: docstring to include the graph driver exception base you chose to catch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cognee/tasks/chunks/create_chunk_associations.py`:
- Around line 131-134: The except block that catches (OSError, ValueError,
RuntimeError) around graph_engine.add_edges() in create_chunk_associations.py
misses backend-specific driver exceptions and the docstring's Raises list is
incomplete; update the try/except to also catch the graph driver's base
exception(s) (or use a broader persistence failure base like Exception if
appropriate for uniform logging/re-raising) so all persistence errors from
graph_engine.add_edges() are logged and re-raised, and update the Raises:
docstring to include the graph driver exception base you chose to catch.
In `@cognee/tasks/codingagents/coding_rule_associations.py`:
- Around line 68-86: Remove the dead TypeError handler around the rule
processing: drop the try/except and instead explicitly check whether rule_id =
getattr(rule, "id", None) is None and skip appending to relationships when it
is, logging the skip via logger.info if desired; leave all other operations
(tuple/dict construction and relationships.append) unguarded so real errors from
rule access surface normally, referencing the variables rule, rule_id,
origin_id, relationships, and logger in your change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b49749fa-bab7-480f-93b4-003439990412
📒 Files selected for processing (3)
cognee/shared/logging_utils.pycognee/tasks/chunks/create_chunk_associations.pycognee/tasks/codingagents/coding_rule_associations.py
🚧 Files skipped from review as they are similar to previous changes (1)
- cognee/shared/logging_utils.py
1 participant
Fixes #2318
Summary
This PR narrows broad
except Exceptionandexcept:handlers in priority files to catch specific exception types (OSError, ValueError, RuntimeError) instead. broadException, allowing unexpected bugs to bubble up properly while maintaining graceful error handling for expected I/O errors, query errors, and runtime errors.Updated docstrings to reflect the specific exceptions are raised instead of generic
Exception.cognee/tasks/codingagents/coding_rule_associations.py: Narrowedexcept Exceptiontoexcept TypeErrorsince the only likely exception from attribute access or tuple construction isTypeError.cognee/shared/logging_utils.py: Replaced bareexcept:clauses withexcept OSErrorfor file system operations. directory creation and permission checks.These changes align with PEP8 recommendations by:
Testing
Tests were to pass without modification. Tests only verify syntax.
Files Changed
cognee/shared/logging_utils.pycognee/tasks/chunks/create_chunk_associations.pycognee/tasks/codingagents/coding_rule_associations.pyDiff stats:
cognee/shared/logging_utils.py | 5++---
cognee/tasks/chunks/create_chunk_associations.py | 12 +++++++---
cognee/tasks/codingagents/coding_rule_associations.py | 2 +-
3 files changed, 10 insertions(+), 14 delet(-)
Summary by CodeRabbit