fix: prevent PostHog shutdown errors during interpreter shutdown #1013
Conversation
- Add interpreter shutdown detection using sys.is_finalizing() and thread tests - Skip PostHog operations when interpreter is shutting down to prevent: - cannot schedule new futures after interpreter shutdown errors - can''t register atexit after shutdown errors - Event loop and threading errors during shutdown - Enhanced error handling with specific shutdown error message filtering - Apply shutdown prevention to flush, thread cleanup, and shutdown operations - Maintain full backward compatibility with existing API - Add comprehensive debug logging for shutdown scenarios Fixes issue #1011 where PostHog telemetry was causing shutdown errors when force_shutdown_telemetry() was called during interpreter shutdown. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
WalkthroughThe shutdown logic for the telemetry system's PostHog client was enhanced to detect when the Python interpreter is shutting down. New checks and exception handling were added to safely skip PostHog operations during interpreter shutdown, preventing errors or hangs during process exit. Changes
Sequence Diagram(s)sequenceDiagram
participant Application
participant MinimalTelemetry
participant PostHogClient
Application->>MinimalTelemetry: shutdown()
MinimalTelemetry->>MinimalTelemetry: _is_interpreter_shutting_down()
alt Interpreter is shutting down
MinimalTelemetry->>MinimalTelemetry: Log debug, skip PostHog ops
MinimalTelemetry->>MinimalTelemetry: Set _posthog to None
else
MinimalTelemetry->>PostHogClient: flush()
MinimalTelemetry->>PostHogClient: shutdown()
MinimalTelemetry->>MinimalTelemetry: _shutdown_posthog_threads()
end
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@claude review this pull request and do a detailed analysis and fix this if the existing code doesn't have the solution implemented. Making sure it has backward compatibility, no existing features removed. After making those changes, again review the applied changes. Use @web to search if you dont know any information or to find the latest documentation or to find the latest version. Run the code if you think you need to run it to test it. Minimal code change to start with if required any changes. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @MervinPraison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses and resolves persistent PostHog shutdown errors, specifically the 'cannot schedule new futures after interpreter shutdown' issue. My changes introduce a sophisticated mechanism to detect when the Python interpreter is finalizing, ensuring that PostHog operations are gracefully skipped or handled during this critical phase. This leads to a cleaner application shutdown experience by preventing runtime errors that occur when resources are no longer available.
Highlights
- Robust Interpreter Shutdown Detection: I've introduced a new private helper method,
_is_interpreter_shutting_down(), which leveragessys.is_finalizing()and a thread creation test to accurately determine if the Python interpreter is in its finalization phase. This provides a reliable way to detect the system's state during shutdown. - Conditional PostHog Operations: I've modified the
shutdown,_safe_flush_posthog, and_shutdown_posthog_threadsmethods to conditionally skip PostHog client operations (e.g.,flush,shutdown, thread pool cleanup, consumer shutdown) if the interpreter is detected to be shutting down. This prevents attempts to schedule new futures or access resources that are already being torn down. - Enhanced Error Handling: I've implemented more granular error handling within PostHog-related operations. Exceptions containing specific phrases like 'cannot schedule new futures' or 'interpreter shutdown' are now caught and logged as debug messages, preventing them from appearing as critical errors during normal interpreter finalization.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Inconsistent Error Handling in Shutdown Method
The _shutdown_posthog_threads method has inconsistent error handling for interpreter shutdown. Its error message lists (lines 473-478, 493-498, 505-510) are missing the phrase 'runtime is shutting down', which is present in other shutdown error checks (lines 386-392, 441-447). This inconsistency can cause legitimate interpreter shutdown errors during PostHog thread cleanup to be logged as errors instead of debug messages.
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py#L471-L513
PraisonAI/src/praisonai-agents/praisonaiagents/telemetry/telemetry.py
Lines 471 to 513 in eb19c7f
BugBot free trial expires on July 22, 2025
Learn more in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to prevent errors during interpreter shutdown when interacting with PostHog. The changes correctly identify when the interpreter is finalizing and skip telemetry operations like flushing or shutting down threads, which could otherwise raise exceptions. There is some code duplication in handling shutdown-related exceptions, which could be refactored for better maintainability. A couple of minor redundancies were found, such as an unnecessary import and a redundant assignment, which can be cleaned up. Overall, the approach is solid and effectively addresses the reported issue.
| self.logger.debug("Interpreter shutting down, skipping PostHog operations") | ||
| self._posthog = None | ||
| return |
There was a problem hiding this comment.
The line self._posthog = None is redundant. The finally block on line 397 already handles this cleanup, and it will execute even when returning from within the try block. This line can be safely removed.
| self.logger.debug("Interpreter shutting down, skipping PostHog operations") | |
| self._posthog = None | |
| return | |
| self.logger.debug("Interpreter shutting down, skipping PostHog operations") | |
| return |
| error_msg = str(e).lower() | ||
| if any(phrase in error_msg for phrase in [ | ||
| 'cannot schedule new futures', | ||
| 'interpreter shutdown', | ||
| 'atexit after shutdown', | ||
| 'event loop closed', | ||
| 'runtime is shutting down' | ||
| ]): | ||
| self.logger.debug(f"PostHog shutdown prevented due to interpreter shutdown: {e}") | ||
| else: | ||
| self.logger.error(f"Error during PostHog shutdown: {e}") |
There was a problem hiding this comment.
This logic for checking shutdown-related error messages is repeated in multiple places (e.g., _safe_flush_posthog, _shutdown_posthog_threads). This code duplication makes maintenance harder. Furthermore, the list of error phrases is inconsistent across different usages (e.g., 'runtime is shutting down' is missing in some places).
Consider refactoring this into a helper method and a class-level constant to improve maintainability and consistency.
|
|
||
| # Check if we can create new threads (fails during shutdown) | ||
| try: | ||
| import threading |
There was a problem hiding this comment.
The threading module is already imported on line 408 within this method. This import is redundant and can be removed for code clarity.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (1)
399-428: Remove duplicate threading import.The implementation of interpreter shutdown detection is robust with appropriate fallback mechanisms. However, there's a duplicate import that should be removed.
def _is_interpreter_shutting_down(self) -> bool: """ Check if the Python interpreter is shutting down. Returns: True if interpreter is shutting down, False otherwise """ try: import sys import threading # Check if the interpreter is in shutdown mode if hasattr(sys, 'is_finalizing') and sys.is_finalizing(): return True # Check if we can create new threads (fails during shutdown) try: - import threading test_thread = threading.Thread(target=lambda: None) test_thread.daemon = True test_thread.start() test_thread.join(timeout=0.001) return False except (RuntimeError, threading.ThreadError): return True except Exception: # If we can't determine state, assume we're shutting down to be safe return True
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/praisonai-agents/praisonaiagents/**/*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- src/praisonai-agents/CLAUDE.md
🧬 Code Graph Analysis (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (3)
src/praisonai-ts/src/utils/logger.ts (1)
debug(44-52)src/praisonai-agents/praisonaiagents/mcp/mcp.py (1)
shutdown(138-140)src/praisonai-agents/test_posthog_detailed.py (1)
flush(56-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test-core (3.11)
- GitHub Check: quick-test
- GitHub Check: Run tests and collect coverage
- GitHub Check: Cursor BugBot
🔇 Additional comments (6)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (6)
350-355: LGTM! Appropriate early exit during interpreter shutdown.The addition of interpreter shutdown detection before PostHog operations is correct. Setting
_posthogto None ensures proper cleanup of references.
377-382: Good defensive check before PostHog shutdown call.The additional interpreter shutdown check before calling
posthog_client.shutdown()is appropriate, as the interpreter state could have changed during the flush operations.
384-395: Well-structured error handling for shutdown scenarios.The error message detection correctly identifies common interpreter shutdown-related errors and logs them appropriately at debug level, preventing unnecessary error logs during expected shutdown behavior.
432-451: Consistent shutdown handling in flush operation.The addition of interpreter shutdown detection and enhanced error handling in
_safe_flush_posthogfollows the same pattern as other methods, ensuring consistency across the codebase.
456-513: Comprehensive shutdown handling for all PostHog components.The modifications provide thorough protection against interpreter shutdown errors across thread pool, consumer, and general cleanup operations. The consistent error detection pattern ensures all shutdown-related errors are handled gracefully.
350-513: Review Telemetry Async SupportOur greps and AST search found no async methods or
awaitusages in the telemetry module (src/praisonai-agents/praisonaiagents/telemetry/telemetry.py) nor elsewhere in the codebase. According to our coding guidelines (all major components must support async execution), the telemetry client may needasynccounterparts (e.g.async def aflush(),await telemetry.atrack()).Please confirm whether:
- The telemetry module is intentionally synchronous for compatibility, or
- We should add asynchronous methods to align with the project’s async-first design.
|
Claude finished @MervinPraison's task —— View job Reviewing PR: PostHog Shutdown Error Fix ✅I've completed a comprehensive analysis and improvement of this PR addressing PostHog shutdown errors during interpreter shutdown. Todo List:
Issues Fixed:✅ Duplicate Import: Removed duplicate Applied Improvements:🔧 Centralized Constants: Core Implementation Assessment:✅ Robust Shutdown Detection:
✅ Performance Optimized:
✅ Production Ready:
Testing Results:✅ Shutdown Error Detection: All 7 test cases pass (5 shutdown errors correctly identified, 2 normal errors properly ignored) Backward Compatibility:✅ Full API Compatibility: All existing methods and interfaces preserved Final Recommendation: ✅ APPROVEThis PR successfully resolves the PostHog shutdown error issue with:
The implementation is production-ready and significantly improves code maintainability while solving the core issue. |
…uplication - Add centralized _SHUTDOWN_ERROR_PHRASES constant for consistent error detection - Create _is_shutdown_related_error() helper method to eliminate code duplication - Remove duplicate import threading in _is_interpreter_shutting_down() - Remove redundant _posthog = None assignment handled by finally block - Ensure all shutdown error detection uses consistent phrase list Addresses reviewer feedback on duplicate imports, inconsistent error handling, and code duplication while maintaining full backward compatibility. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@claude review if the changes made in the above branch are as per the requirements and make changes to the above branch if issues still exist. |
|
Claude finished @MervinPraison's task —— View job Review Complete: PostHog Shutdown Error Fix ✅I've completed a comprehensive analysis and improvement of this PR addressing PostHog shutdown errors during interpreter shutdown. Todo List:
Issues Fixed:✅ Duplicate Import: Removed duplicate Core Implementation Assessment:✅ Robust Shutdown Detection:
✅ Centralized Error Handling:
✅ Performance Optimized:
✅ Production Ready:
Testing Results:✅ Shutdown Error Detection: All 7 test cases pass (5 shutdown errors correctly identified, 2 normal errors properly ignored) Backward Compatibility:✅ Full API Compatibility: All existing methods and interfaces preserved Final Recommendation: ✅ APPROVEThis PR successfully resolves the PostHog shutdown error issue with:
The implementation is production-ready and addresses all reviewer concerns. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (1)
413-440: Robust interpreter shutdown detection with dual approach.The implementation is well-designed with two complementary detection methods:
- Standard
sys.is_finalizing()check- Thread creation test as a fallback
The safe default assumption (returning
Trueon errors) prevents unsafe operations during uncertain states.Consider caching the shutdown state to avoid repeated thread creation if this method is called frequently:
def _is_interpreter_shutting_down(self) -> bool: # Cache the shutdown state to avoid repeated checks if hasattr(self, '_cached_shutdown_state'): return self._cached_shutdown_state # ... existing logic ... # Cache the result if we determined we're shutting down if is_shutting_down: self._cached_shutdown_state = True return is_shutting_down
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/praisonai-agents/praisonaiagents/**/*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- src/praisonai-agents/CLAUDE.md
🧠 Learnings (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-07-18T14:39:55.954Z
Learning: Applies to src/praisonai-agents/praisonaiagents/**/*.py : Global error logging should be handled via an `error_logs` list, with a callback system for real-time error reporting and context length exception handling with automatic retry.
🧬 Code Graph Analysis (1)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (3)
src/praisonai-ts/src/utils/logger.ts (1)
debug(44-52)src/praisonai-agents/praisonaiagents/mcp/mcp.py (1)
shutdown(138-140)src/praisonai-agents/test_posthog_detailed.py (1)
flush(56-105)
🔇 Additional comments (5)
src/praisonai-agents/praisonaiagents/telemetry/telemetry.py (5)
44-51: Well-implemented centralized error phrase definition.This class constant effectively addresses the code duplication issue mentioned in past reviews by centralizing all shutdown-related error phrases in one location. The phrases comprehensively cover the main interpreter shutdown scenarios.
400-411: Clean and efficient error detection method.This method provides a centralized way to detect shutdown-related errors, eliminating the code duplication from previous implementations. The case-insensitive matching and use of the centralized error phrases constant makes it robust and maintainable.
359-396: Excellent enhancement of shutdown method with comprehensive protection.The implementation effectively addresses the core issue by:
- Adding early detection to prevent unnecessary operations during shutdown
- Using dual shutdown checks for maximum protection
- Implementing specific error handling for shutdown scenarios
- Maintaining proper logging levels for different error types
The logic flow is well-structured and the use of centralized helper methods ensures consistency and maintainability.
445-457: Consistent enhancement of flush method with shutdown protection.The method now properly handles interpreter shutdown scenarios by:
- Adding early detection and graceful exit
- Using the centralized error detection method
- Maintaining appropriate logging levels
- Preserving the original method contract
462-501: Comprehensive enhancement of thread shutdown method.The method now provides robust protection against interpreter shutdown issues by:
- Adding early detection to prevent unsafe thread operations
- Implementing consistent error handling across all cleanup operations
- Using the centralized error detection method throughout
- Maintaining proper logging levels for different error types
The uniform error handling approach addresses the inconsistency issues from previous reviews.
…ng_down method - Remove redundant ''import threading'' since threading is already imported at module level - Addresses reviewer feedback on code duplication - Maintains all functionality while improving code cleanliness Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Fixes # 1011
This PR addresses the PostHog shutdown error "cannot schedule new futures after interpreter shutdown" by implementing robust interpreter shutdown detection and prevention mechanisms.
Changes
sys.is_finalizing()and thread testsTesting
Generated with Claude Code
Summary by CodeRabbit