mirror of
https://github.com/crewAIInc/crewAI.git
synced 2025-12-16 04:18:35 +00:00
fix: handle missing signals on Windows (SIGHUP, SIGTSTP, SIGCONT)
This fixes GitHub issue #4062 where crewai crashes on Windows with 'module signal has no attribute SIGHUP' error. Changes: - Use getattr with fallback values for signals not available on Windows - Make telemetry signal registration conditional on signal availability - Add regression tests for Windows compatibility The fix maintains backward compatibility on Unix systems while allowing crewai to work on Windows where SIGHUP, SIGTSTP, and SIGCONT are not available. Co-Authored-By: João <joao@crewai.com>
This commit is contained in:
@@ -14,14 +14,25 @@ from pydantic import Field, TypeAdapter
|
|||||||
from crewai.events.base_events import BaseEvent
|
from crewai.events.base_events import BaseEvent
|
||||||
|
|
||||||
|
|
||||||
|
# Fallback values for signals not available on Windows.
|
||||||
|
# These are negative to avoid conflicts with real signal numbers.
|
||||||
|
_FALLBACK_SIGHUP = -1
|
||||||
|
_FALLBACK_SIGTSTP = -2
|
||||||
|
_FALLBACK_SIGCONT = -3
|
||||||
|
|
||||||
|
|
||||||
class SignalType(IntEnum):
|
class SignalType(IntEnum):
|
||||||
"""Enumeration of supported system signals."""
|
"""Enumeration of supported system signals.
|
||||||
|
|
||||||
|
Note: SIGHUP, SIGTSTP, and SIGCONT are not available on Windows.
|
||||||
|
On Windows, these will use fallback negative values.
|
||||||
|
"""
|
||||||
|
|
||||||
SIGTERM = signal.SIGTERM
|
SIGTERM = signal.SIGTERM
|
||||||
SIGINT = signal.SIGINT
|
SIGINT = signal.SIGINT
|
||||||
SIGHUP = signal.SIGHUP
|
SIGHUP = getattr(signal, "SIGHUP", _FALLBACK_SIGHUP)
|
||||||
SIGTSTP = signal.SIGTSTP
|
SIGTSTP = getattr(signal, "SIGTSTP", _FALLBACK_SIGTSTP)
|
||||||
SIGCONT = signal.SIGCONT
|
SIGCONT = getattr(signal, "SIGCONT", _FALLBACK_SIGCONT)
|
||||||
|
|
||||||
|
|
||||||
class SigTermEvent(BaseEvent):
|
class SigTermEvent(BaseEvent):
|
||||||
|
|||||||
@@ -172,11 +172,21 @@ class Telemetry:
|
|||||||
|
|
||||||
self._original_handlers: dict[int, Any] = {}
|
self._original_handlers: dict[int, Any] = {}
|
||||||
|
|
||||||
|
# Always-supported signals on all platforms
|
||||||
self._register_signal_handler(signal.SIGTERM, SigTermEvent, shutdown=True)
|
self._register_signal_handler(signal.SIGTERM, SigTermEvent, shutdown=True)
|
||||||
self._register_signal_handler(signal.SIGINT, SigIntEvent, shutdown=True)
|
self._register_signal_handler(signal.SIGINT, SigIntEvent, shutdown=True)
|
||||||
self._register_signal_handler(signal.SIGHUP, SigHupEvent, shutdown=False)
|
|
||||||
self._register_signal_handler(signal.SIGTSTP, SigTStpEvent, shutdown=False)
|
# Optional signals (not available on Windows)
|
||||||
self._register_signal_handler(signal.SIGCONT, SigContEvent, shutdown=False)
|
optional_signals: list[tuple[str, type, bool]] = [
|
||||||
|
("SIGHUP", SigHupEvent, False),
|
||||||
|
("SIGTSTP", SigTStpEvent, False),
|
||||||
|
("SIGCONT", SigContEvent, False),
|
||||||
|
]
|
||||||
|
|
||||||
|
for sig_name, event_class, shutdown in optional_signals:
|
||||||
|
sig = getattr(signal, sig_name, None)
|
||||||
|
if sig is not None:
|
||||||
|
self._register_signal_handler(sig, event_class, shutdown=shutdown)
|
||||||
|
|
||||||
def _register_signal_handler(
|
def _register_signal_handler(
|
||||||
self,
|
self,
|
||||||
|
|||||||
@@ -24,12 +24,25 @@ class TestSignalType:
|
|||||||
"""Tests for SignalType enum."""
|
"""Tests for SignalType enum."""
|
||||||
|
|
||||||
def test_signal_type_values(self) -> None:
|
def test_signal_type_values(self) -> None:
|
||||||
"""Verify SignalType maps to correct signal numbers."""
|
"""Verify SignalType maps to correct signal numbers when available."""
|
||||||
assert SignalType.SIGTERM == signal.SIGTERM
|
assert SignalType.SIGTERM == signal.SIGTERM
|
||||||
assert SignalType.SIGINT == signal.SIGINT
|
assert SignalType.SIGINT == signal.SIGINT
|
||||||
assert SignalType.SIGHUP == signal.SIGHUP
|
|
||||||
assert SignalType.SIGTSTP == signal.SIGTSTP
|
# These signals are not available on Windows, so only check if present
|
||||||
assert SignalType.SIGCONT == signal.SIGCONT
|
if hasattr(signal, "SIGHUP"):
|
||||||
|
assert SignalType.SIGHUP == signal.SIGHUP
|
||||||
|
if hasattr(signal, "SIGTSTP"):
|
||||||
|
assert SignalType.SIGTSTP == signal.SIGTSTP
|
||||||
|
if hasattr(signal, "SIGCONT"):
|
||||||
|
assert SignalType.SIGCONT == signal.SIGCONT
|
||||||
|
|
||||||
|
def test_signal_type_enum_members_always_exist(self) -> None:
|
||||||
|
"""Verify all SignalType enum members exist regardless of platform."""
|
||||||
|
assert hasattr(SignalType, "SIGTERM")
|
||||||
|
assert hasattr(SignalType, "SIGINT")
|
||||||
|
assert hasattr(SignalType, "SIGHUP")
|
||||||
|
assert hasattr(SignalType, "SIGTSTP")
|
||||||
|
assert hasattr(SignalType, "SIGCONT")
|
||||||
|
|
||||||
|
|
||||||
class TestSignalEvents:
|
class TestSignalEvents:
|
||||||
@@ -194,4 +207,71 @@ class TestSignalEventSerialization:
|
|||||||
restored = signal_event_adapter.validate_python(serialized)
|
restored = signal_event_adapter.validate_python(serialized)
|
||||||
assert isinstance(restored, SigTermEvent)
|
assert isinstance(restored, SigTermEvent)
|
||||||
assert restored.reason == original.reason
|
assert restored.reason == original.reason
|
||||||
assert restored.type == original.type
|
assert restored.type == original.type
|
||||||
|
|
||||||
|
|
||||||
|
class TestWindowsCompatibility:
|
||||||
|
"""Tests for Windows compatibility (signals not available on Windows)."""
|
||||||
|
|
||||||
|
def test_system_events_imports_when_optional_signals_missing(
|
||||||
|
self, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
"""system_events should import even if some signals are missing (Windows-like).
|
||||||
|
|
||||||
|
This is a regression test for GitHub issue #4062.
|
||||||
|
"""
|
||||||
|
import importlib
|
||||||
|
|
||||||
|
import crewai.events.types.system_events as system_events_module
|
||||||
|
|
||||||
|
# Simulate a Windows-like signal module by removing optional signals
|
||||||
|
monkeypatch.delattr(signal, "SIGHUP", raising=False)
|
||||||
|
monkeypatch.delattr(signal, "SIGTSTP", raising=False)
|
||||||
|
monkeypatch.delattr(signal, "SIGCONT", raising=False)
|
||||||
|
|
||||||
|
# Reload after patching so class definitions see the modified signal module
|
||||||
|
reloaded = importlib.reload(system_events_module)
|
||||||
|
|
||||||
|
# Import should succeed and enum members should exist
|
||||||
|
assert hasattr(reloaded.SignalType, "SIGHUP")
|
||||||
|
assert hasattr(reloaded.SignalType, "SIGTSTP")
|
||||||
|
assert hasattr(reloaded.SignalType, "SIGCONT")
|
||||||
|
|
||||||
|
# Event classes should still be importable
|
||||||
|
assert hasattr(reloaded, "SigHupEvent")
|
||||||
|
assert hasattr(reloaded, "SigTStpEvent")
|
||||||
|
assert hasattr(reloaded, "SigContEvent")
|
||||||
|
|
||||||
|
# Fallback values should be negative (to avoid conflicts with real signals)
|
||||||
|
assert reloaded.SignalType.SIGHUP < 0
|
||||||
|
assert reloaded.SignalType.SIGTSTP < 0
|
||||||
|
assert reloaded.SignalType.SIGCONT < 0
|
||||||
|
|
||||||
|
def test_signal_events_can_be_created_when_signals_missing(
|
||||||
|
self, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
"""Signal events should be creatable even when signals are missing.
|
||||||
|
|
||||||
|
This is a regression test for GitHub issue #4062.
|
||||||
|
"""
|
||||||
|
import importlib
|
||||||
|
|
||||||
|
import crewai.events.types.system_events as system_events_module
|
||||||
|
|
||||||
|
# Simulate a Windows-like signal module
|
||||||
|
monkeypatch.delattr(signal, "SIGHUP", raising=False)
|
||||||
|
monkeypatch.delattr(signal, "SIGTSTP", raising=False)
|
||||||
|
monkeypatch.delattr(signal, "SIGCONT", raising=False)
|
||||||
|
|
||||||
|
# Reload after patching
|
||||||
|
reloaded = importlib.reload(system_events_module)
|
||||||
|
|
||||||
|
# Events should be creatable
|
||||||
|
hup_event = reloaded.SigHupEvent()
|
||||||
|
assert hup_event.type == "SIGHUP"
|
||||||
|
|
||||||
|
tstp_event = reloaded.SigTStpEvent()
|
||||||
|
assert tstp_event.type == "SIGTSTP"
|
||||||
|
|
||||||
|
cont_event = reloaded.SigContEvent()
|
||||||
|
assert cont_event.type == "SIGCONT"
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import os
|
import os
|
||||||
|
import signal
|
||||||
import threading
|
import threading
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
@@ -121,3 +122,36 @@ def test_telemetry_singleton_pattern():
|
|||||||
thread.join()
|
thread.join()
|
||||||
|
|
||||||
assert all(instance is telemetry1 for instance in instances)
|
assert all(instance is telemetry1 for instance in instances)
|
||||||
|
|
||||||
|
|
||||||
|
def test_telemetry_register_shutdown_handlers_with_missing_optional_signals(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
"""Telemetry shouldn't fail when optional signals are missing (Windows-like).
|
||||||
|
|
||||||
|
This is a regression test for GitHub issue #4062.
|
||||||
|
"""
|
||||||
|
import importlib
|
||||||
|
|
||||||
|
from crewai.telemetry import telemetry as telemetry_module
|
||||||
|
|
||||||
|
# Disable telemetry to avoid real OTLP setup
|
||||||
|
monkeypatch.setenv("CREWAI_DISABLE_TELEMETRY", "true")
|
||||||
|
|
||||||
|
# Simulate a Windows-like signal module by removing optional signals
|
||||||
|
monkeypatch.delattr(signal, "SIGHUP", raising=False)
|
||||||
|
monkeypatch.delattr(signal, "SIGTSTP", raising=False)
|
||||||
|
monkeypatch.delattr(signal, "SIGCONT", raising=False)
|
||||||
|
|
||||||
|
# Reload after patching so the module sees the modified signal module
|
||||||
|
reloaded = importlib.reload(telemetry_module)
|
||||||
|
|
||||||
|
# Reset the singleton to allow a new instance
|
||||||
|
reloaded.Telemetry._instance = None
|
||||||
|
reloaded.Telemetry._lock = threading.Lock()
|
||||||
|
|
||||||
|
# This should not raise an error even with missing signals
|
||||||
|
telemetry = reloaded.Telemetry()
|
||||||
|
|
||||||
|
# Telemetry should be disabled (due to env var), but import should succeed
|
||||||
|
assert telemetry.ready is False
|
||||||
|
|||||||
Reference in New Issue
Block a user