-
Notifications
You must be signed in to change notification settings - Fork 0
chore(SESF-45): scrub pre-existing exception logs in rag_engine.py #41
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
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 |
|---|---|---|
|
|
@@ -394,7 +394,7 @@ def close_server_mode(): | |
| client.close() | ||
| logger.info("Closed Milvus client: %s", path) | ||
| except Exception as e: | ||
| logger.warning("Error closing Milvus client %s: %s", path, e) | ||
| logger.warning("Error closing Milvus client %s: %s", path, _scrub_exception(e)) | ||
| _persistent_clients.clear() | ||
| _fts.close_all() | ||
| # Nil the semaphore and lock FIRST so any coroutine that wakes up while we | ||
|
|
@@ -416,7 +416,7 @@ def _get_persistent_client(db_path: str) -> MilvusClient: | |
| _persistent_clients[db_path].has_collection(COLLECTION_NAME) | ||
| return _persistent_clients[db_path] | ||
| except Exception as e: | ||
| logger.warning("Stale Milvus client for %s: %s — reconnecting", db_path, e) | ||
| logger.warning("Stale Milvus client for %s: %s — reconnecting", db_path, _scrub_exception(e)) | ||
| try: | ||
| _persistent_clients[db_path].close() | ||
| except Exception: | ||
|
|
@@ -441,7 +441,7 @@ def _get_persistent_client(db_path: str) -> MilvusClient: | |
| ) | ||
| logger.info("Opened client: %s", db_path) | ||
| except Exception as e: | ||
| logger.error("Failed to connect to Milvus at %s: %s", db_path, e) | ||
| logger.error("Failed to connect to Milvus at %s: %s", db_path, _scrub_exception(e)) | ||
| raise | ||
| return _persistent_clients[db_path] | ||
|
|
||
|
|
@@ -834,7 +834,7 @@ def add_turns(turns: List[Dict], db_path: Optional[str] = None) -> int: | |
| if results: | ||
| existing_ids.add(doc_id) | ||
| except Exception as e: | ||
| logger.warning("Dedup check failed for doc_id %s: %s", doc_id, e) | ||
| logger.warning("Dedup check failed for doc_id %s: %s", doc_id, _scrub_exception(e)) | ||
|
|
||
| new_turns = [t for t in turns if t["doc_id"] not in existing_ids] | ||
| if not new_turns: | ||
|
|
@@ -1537,7 +1537,7 @@ def _passes(entry: Dict) -> bool: | |
| filters=fts_filters or None, db_path=db_path, | ||
| ) | ||
| except Exception as e: | ||
| logger.warning("FTS fallback failed for issue timeline %s (non-fatal): %s", canonical, e) | ||
| logger.warning("FTS fallback failed for issue timeline %s (non-fatal): %s", canonical, _scrub_exception(e)) | ||
| fts_hits = [] | ||
|
|
||
| for row in fts_hits: | ||
|
|
@@ -1951,7 +1951,7 @@ def delete_by_session(session_id: str, db_path: Optional[str] = None) -> int: | |
| _fts.delete(conn, "session_id", session_id) | ||
| _fts.close_ephemeral(conn) | ||
| except Exception as e: | ||
| logger.warning("FTS delete by session failed (non-fatal): %s", e) | ||
| logger.warning("FTS delete by session failed (non-fatal): %s", _scrub_exception(e)) | ||
|
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. 🔴 HIGH RISK The ephemeral FTS connection is not closed if an exception occurs during deletion, resulting in a resource leak. The block should use a |
||
|
|
||
| return len(results) | ||
|
|
||
|
|
@@ -2217,7 +2217,7 @@ def delete_by_branch(git_branch: str, db_path: Optional[str] = None) -> int: | |
| _fts.delete(conn, "git_branch", git_branch) | ||
| _fts.close_ephemeral(conn) | ||
| except Exception as e: | ||
| logger.warning("FTS delete by branch failed (non-fatal): %s", e) | ||
| logger.warning("FTS delete by branch failed (non-fatal): %s", _scrub_exception(e)) | ||
|
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. 🔴 HIGH RISK The ephemeral FTS connection is not closed if an exception occurs during deletion, resulting in a resource leak. Use a |
||
|
|
||
| return len(results) | ||
|
|
||
|
|
@@ -2256,7 +2256,7 @@ def delete_older_than(max_age_days: int, db_path: Optional[str] = None) -> int: | |
| _fts.delete_where(conn, "timestamp < ? AND timestamp != ''", (cutoff_str,)) | ||
| _fts.close_ephemeral(conn) | ||
| except Exception as e: | ||
| logger.warning("FTS delete older_than failed (non-fatal): %s", e) | ||
| logger.warning("FTS delete older_than failed (non-fatal): %s", _scrub_exception(e)) | ||
|
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. 🔴 HIGH RISK The ephemeral FTS connection is not closed if an exception occurs during deletion, resulting in a resource leak. Use a |
||
|
|
||
| return len(results) | ||
|
|
||
|
|
||
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.
🟡 MEDIUM RISK
Suggestion: While the local log is now scrubbed, re-raising the exception
ewithout modification means any upstream logger will still record the sensitive data. Additionally,db_pathmay contain credentials that should be redacted before logging. Update the logging to redact thedb_pathargument and mutatee.argsto redact secrets before re-raising.