diff --git a/bug_report.md b/bug_report.md new file mode 100644 index 00000000..7c7c527a --- /dev/null +++ b/bug_report.md @@ -0,0 +1,170 @@ +# Bug Report: Swarms Codebase Issues + +## Bug 1: Error Handling in Daemon Thread (Critical) + +**Location:** `swarms/structs/agent.py` lines 1446-1453 + +**Description:** The `_handle_run_error` method creates a daemon thread to handle errors, but the exception raised in `__handle_run_error` will not propagate back to the main thread. This causes silent failures where errors are logged but not properly propagated to the caller. + +**Type:** Concurrency/Error Handling Bug + +**Severity:** Critical - Can lead to silent failures + +**Current Code:** +```python +def _handle_run_error(self, error: any): + process_thread = threading.Thread( + target=self.__handle_run_error, + args=(error,), + daemon=True, + ) + process_thread.start() +``` + +**Problem:** +- The daemon thread will exit when the main thread exits +- The `raise error` at the end of `__handle_run_error` occurs in the daemon thread, not the main thread +- This means exceptions are lost and not properly handled by the calling code + +**Fix:** Remove the threading wrapper and call the error handler directly, or use proper exception propagation. + +--- + +## Bug 2: Method Name Typo (Logic Error) + +**Location:** `swarms/structs/agent.py` lines 2128 and 2122 + +**Description:** There are two related typos in the response filtering functionality: +1. The method `apply_reponse_filters` has a typo in the name - it should be `apply_response_filters` +2. The `add_response_filter` method accesses `self.reponse_filters` instead of `self.response_filters` + +**Type:** Naming/Logic Error + +**Severity:** Medium - Can cause AttributeError when called + +**Current Code:** +```python +def add_response_filter(self, filter_word: str) -> None: + logger.info(f"Adding response filter: {filter_word}") + self.reponse_filters.append(filter_word) # TYPO: reponse_filters + +def apply_reponse_filters(self, response: str) -> str: # TYPO: apply_reponse_filters + """ + Apply the response filters to the response + """ + logger.info( + f"Applying response filters to response: {response}" + ) + for word in self.response_filters: + response = response.replace(word, "[FILTERED]") + return response +``` + +**Problem:** +- Method name is misspelled: `apply_reponse_filters` instead of `apply_response_filters` +- Attribute access is misspelled: `self.reponse_filters` instead of `self.response_filters` +- The method is called correctly in `filtered_run` method, suggesting these are typos + +**Fix:** Fix both typos to use correct spelling. + +--- + +## Bug 3: Document Ingestion Logic Error (Data Loss) + +**Location:** `swarms/structs/agent.py` lines 2193-2212 + +**Description:** The `ingest_docs` method has a logic error where it processes all documents in a loop but only retains the data from the last document. All previous documents are processed but their data is overwritten and lost. + +**Type:** Logic Error + +**Severity:** High - Causes data loss + +**Current Code:** +```python +def ingest_docs(self, docs: List[str], *args, **kwargs): + """Ingest the docs into the memory + + Args: + docs (List[str]): Documents of pdfs, text, csvs + + Returns: + None + """ + try: + for doc in docs: + data = data_to_text(doc) + + return self.short_memory.add( + role=self.user_name, content=data + ) + except Exception as error: + logger.info(f"Error ingesting docs: {error}", "red") +``` + +**Problem:** +- The `data` variable is overwritten on each iteration +- Only the last document's data is actually added to memory +- All previous documents are processed but their data is lost +- The method should either process documents individually or combine all data + +**Fix:** Accumulate all document data or process each document individually. + +--- + +## Impact Assessment + +1. **Bug 1 (Critical):** Can cause silent failures in production, making debugging difficult +2. **Bug 2 (Medium):** Will cause AttributeError when the method is called correctly +3. **Bug 3 (High):** Causes data loss when ingesting multiple documents + +## Fixes Applied + +### Bug 1 Fix - Error Handling +**Status:** ✅ FIXED + +Changed the `_handle_run_error` method to call `__handle_run_error` directly instead of using a daemon thread: +```python +def _handle_run_error(self, error: any): + # Handle error directly instead of using daemon thread + # to ensure proper exception propagation + self.__handle_run_error(error) +``` + +### Bug 2 Fix - Method Name Typos +**Status:** ✅ FIXED + +Fixed both typos in the response filtering functionality: +1. Renamed `apply_reponse_filters` to `apply_response_filters` +2. Fixed `self.reponse_filters` to `self.response_filters` + +### Bug 3 Fix - Document Ingestion Logic +**Status:** ✅ FIXED + +Modified the `ingest_docs` method to process all documents and combine their content: +```python +def ingest_docs(self, docs: List[str], *args, **kwargs): + try: + # Process all documents and combine their content + all_data = [] + for doc in docs: + data = data_to_text(doc) + all_data.append(f"Document: {doc}\n{data}") + + # Combine all document content + combined_data = "\n\n".join(all_data) + + return self.short_memory.add( + role=self.user_name, content=combined_data + ) + except Exception as error: + logger.info(f"Error ingesting docs: {error}", "red") +``` + +## Recommendations + +1. ✅ Fixed the error handling to properly propagate exceptions +2. ✅ Corrected the method name typos +3. ✅ Fixed the document ingestion logic to process all documents +4. Add unit tests to prevent similar issues in the future +5. Consider adding linting rules to catch method name typos +6. Consider code review processes to catch similar issues \ No newline at end of file diff --git a/swarms/structs/agent.py b/swarms/structs/agent.py index b3eddf6f..3ab1573c 100644 --- a/swarms/structs/agent.py +++ b/swarms/structs/agent.py @@ -1444,12 +1444,9 @@ class Agent: raise error def _handle_run_error(self, error: any): - process_thread = threading.Thread( - target=self.__handle_run_error, - args=(error,), - daemon=True, - ) - process_thread.start() + # Handle error directly instead of using daemon thread + # to ensure proper exception propagation + self.__handle_run_error(error) async def arun( self, @@ -2123,9 +2120,9 @@ class Agent: """ logger.info(f"Adding response filter: {filter_word}") - self.reponse_filters.append(filter_word) + self.response_filters.append(filter_word) - def apply_reponse_filters(self, response: str) -> str: + def apply_response_filters(self, response: str) -> str: """ Apply the response filters to the response @@ -2200,11 +2197,17 @@ class Agent: None """ try: + # Process all documents and combine their content + all_data = [] for doc in docs: data = data_to_text(doc) + all_data.append(f"Document: {doc}\n{data}") + + # Combine all document content + combined_data = "\n\n".join(all_data) return self.short_memory.add( - role=self.user_name, content=data + role=self.user_name, content=combined_data ) except Exception as error: logger.info(f"Error ingesting docs: {error}", "red")