Skip to content

Commit 36160c7

Browse files
refactor: improve telemetry shutdown error handling and reduce code duplication
- 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>
1 parent eb19c7f commit 36160c7

File tree

1 file changed

+27
-39
lines changed

1 file changed

+27
-39
lines changed

src/praisonai-agents/praisonaiagents/telemetry/telemetry.py

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ class MinimalTelemetry:
4141
- Can be disabled via environment variables
4242
"""
4343

44+
# Common error phrases that indicate interpreter shutdown
45+
_SHUTDOWN_ERROR_PHRASES = [
46+
'cannot schedule new futures',
47+
'interpreter shutdown',
48+
'atexit after shutdown',
49+
'event loop closed',
50+
'runtime is shutting down'
51+
]
52+
4453
def __init__(self, enabled: bool = None):
4554
"""
4655
Initialize the minimal telemetry collector.
@@ -350,7 +359,6 @@ def shutdown(self):
350359
# Check if Python interpreter is shutting down
351360
if self._is_interpreter_shutting_down():
352361
self.logger.debug("Interpreter shutting down, skipping PostHog operations")
353-
self._posthog = None
354362
return
355363

356364
# Use a timeout-based flush to prevent hanging
@@ -382,20 +390,26 @@ def shutdown(self):
382390

383391
except Exception as e:
384392
# Handle specific shutdown-related errors gracefully
385-
error_msg = str(e).lower()
386-
if any(phrase in error_msg for phrase in [
387-
'cannot schedule new futures',
388-
'interpreter shutdown',
389-
'atexit after shutdown',
390-
'event loop closed',
391-
'runtime is shutting down'
392-
]):
393+
if self._is_shutdown_related_error(e):
393394
self.logger.debug(f"PostHog shutdown prevented due to interpreter shutdown: {e}")
394395
else:
395396
self.logger.error(f"Error during PostHog shutdown: {e}")
396397
finally:
397398
self._posthog = None
398399

400+
def _is_shutdown_related_error(self, error: Exception) -> bool:
401+
"""
402+
Check if an error is related to interpreter shutdown.
403+
404+
Args:
405+
error: The exception to check
406+
407+
Returns:
408+
True if the error is shutdown-related, False otherwise
409+
"""
410+
error_msg = str(error).lower()
411+
return any(phrase in error_msg for phrase in self._SHUTDOWN_ERROR_PHRASES)
412+
399413
def _is_interpreter_shutting_down(self) -> bool:
400414
"""
401415
Check if the Python interpreter is shutting down.
@@ -413,7 +427,6 @@ def _is_interpreter_shutting_down(self) -> bool:
413427

414428
# Check if we can create new threads (fails during shutdown)
415429
try:
416-
import threading
417430
test_thread = threading.Thread(target=lambda: None)
418431
test_thread.daemon = True
419432
test_thread.start()
@@ -437,14 +450,7 @@ def _safe_flush_posthog(self, posthog_client):
437450
posthog_client.flush()
438451
return True
439452
except Exception as e:
440-
error_msg = str(e).lower()
441-
if any(phrase in error_msg for phrase in [
442-
'cannot schedule new futures',
443-
'interpreter shutdown',
444-
'atexit after shutdown',
445-
'event loop closed',
446-
'runtime is shutting down'
447-
]):
453+
if self._is_shutdown_related_error(e):
448454
self.logger.debug(f"PostHog flush prevented due to interpreter shutdown: {e}")
449455
else:
450456
self.logger.debug(f"PostHog flush error: {e}")
@@ -469,13 +475,7 @@ def _shutdown_posthog_threads(self, posthog_client):
469475
import time
470476
time.sleep(0.5)
471477
except Exception as e:
472-
error_msg = str(e).lower()
473-
if any(phrase in error_msg for phrase in [
474-
'cannot schedule new futures',
475-
'interpreter shutdown',
476-
'atexit after shutdown',
477-
'event loop closed'
478-
]):
478+
if self._is_shutdown_related_error(e):
479479
self.logger.debug(f"Thread pool shutdown prevented due to interpreter shutdown: {e}")
480480
else:
481481
self.logger.debug(f"Thread pool shutdown error: {e}")
@@ -489,25 +489,13 @@ def _shutdown_posthog_threads(self, posthog_client):
489489
if hasattr(consumer, 'shutdown'):
490490
consumer.shutdown()
491491
except Exception as e:
492-
error_msg = str(e).lower()
493-
if any(phrase in error_msg for phrase in [
494-
'cannot schedule new futures',
495-
'interpreter shutdown',
496-
'atexit after shutdown',
497-
'event loop closed'
498-
]):
492+
if self._is_shutdown_related_error(e):
499493
self.logger.debug(f"Consumer shutdown prevented due to interpreter shutdown: {e}")
500494
else:
501495
self.logger.debug(f"Consumer shutdown error: {e}")
502496

503497
except Exception as e:
504-
error_msg = str(e).lower()
505-
if any(phrase in error_msg for phrase in [
506-
'cannot schedule new futures',
507-
'interpreter shutdown',
508-
'atexit after shutdown',
509-
'event loop closed'
510-
]):
498+
if self._is_shutdown_related_error(e):
511499
self.logger.debug(f"PostHog thread cleanup prevented due to interpreter shutdown: {e}")
512500
else:
513501
self.logger.debug(f"Error during PostHog thread cleanup: {e}")

0 commit comments

Comments
 (0)