Skip to content

Commit dcfc2da

Browse files
Fix issue #6037: [Bug]: [Resolver] crashes on main (#6284)
Co-authored-by: Engel Nyst <[email protected]>
1 parent 84e2823 commit dcfc2da

File tree

2 files changed

+111
-7
lines changed

2 files changed

+111
-7
lines changed

openhands/runtime/utils/log_streamer.py

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,26 @@ def __init__(
1717
logFn: Callable,
1818
):
1919
self.log = logFn
20-
self.log_generator = container.logs(stream=True, follow=True)
20+
# Initialize all attributes before starting the thread on this instance
21+
self.stdout_thread = None
22+
self.log_generator = None
2123
self._stop_event = threading.Event()
2224

23-
# Start the stdout streaming thread
24-
self.stdout_thread = threading.Thread(target=self._stream_logs)
25-
self.stdout_thread.daemon = True
26-
self.stdout_thread.start()
25+
try:
26+
self.log_generator = container.logs(stream=True, follow=True)
27+
# Start the stdout streaming thread
28+
self.stdout_thread = threading.Thread(target=self._stream_logs)
29+
self.stdout_thread.daemon = True
30+
self.stdout_thread.start()
31+
except Exception as e:
32+
self.log('error', f'Failed to initialize log streaming: {e}')
2733

2834
def _stream_logs(self) -> None:
2935
"""Stream logs from the Docker container to stdout."""
36+
if not self.log_generator:
37+
self.log('error', 'Log generator not initialized')
38+
return
39+
3040
try:
3141
for log_line in self.log_generator:
3242
if self._stop_event.is_set():
@@ -38,7 +48,11 @@ def _stream_logs(self) -> None:
3848
self.log('error', f'Error streaming docker logs to stdout: {e}')
3949

4050
def __del__(self) -> None:
41-
if self.stdout_thread and self.stdout_thread.is_alive():
51+
if (
52+
hasattr(self, 'stdout_thread')
53+
and self.stdout_thread
54+
and self.stdout_thread.is_alive()
55+
):
4256
self.close(timeout=5)
4357

4458
def close(self, timeout: float = 5.0) -> None:
@@ -47,5 +61,5 @@ def close(self, timeout: float = 5.0) -> None:
4761
if self.stdout_thread and self.stdout_thread.is_alive():
4862
self.stdout_thread.join(timeout)
4963
# Close the log generator to release the file descriptor
50-
if hasattr(self.log_generator, 'close'):
64+
if self.log_generator is not None:
5165
self.log_generator.close()

tests/unit/test_log_streamer.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
from unittest.mock import Mock
2+
3+
import pytest
4+
5+
from openhands.runtime.utils.log_streamer import LogStreamer
6+
7+
8+
@pytest.fixture
9+
def mock_container():
10+
return Mock()
11+
12+
13+
@pytest.fixture
14+
def mock_log_fn():
15+
return Mock()
16+
17+
18+
def test_init_failure_handling(mock_container, mock_log_fn):
19+
"""Test that LogStreamer handles initialization failures gracefully."""
20+
mock_container.logs.side_effect = Exception('Test error')
21+
22+
streamer = LogStreamer(mock_container, mock_log_fn)
23+
assert streamer.stdout_thread is None
24+
assert streamer.log_generator is None
25+
mock_log_fn.assert_called_with(
26+
'error', 'Failed to initialize log streaming: Test error'
27+
)
28+
29+
30+
def test_stream_logs_without_generator(mock_container, mock_log_fn):
31+
"""Test that _stream_logs handles missing log generator gracefully."""
32+
streamer = LogStreamer(mock_container, mock_log_fn)
33+
streamer.log_generator = None
34+
streamer._stream_logs()
35+
mock_log_fn.assert_called_with('error', 'Log generator not initialized')
36+
37+
38+
def test_cleanup_without_thread(mock_container, mock_log_fn):
39+
"""Test that cleanup works even if stdout_thread is not initialized."""
40+
streamer = LogStreamer(mock_container, mock_log_fn)
41+
streamer.stdout_thread = None
42+
streamer.close() # Should not raise any exceptions
43+
44+
45+
def test_normal_operation(mock_container, mock_log_fn):
46+
"""Test normal operation of LogStreamer."""
47+
48+
# Create a mock generator class that mimics Docker's log generator
49+
class MockLogGenerator:
50+
def __init__(self, logs):
51+
self.logs = iter(logs)
52+
self.closed = False
53+
54+
def __iter__(self):
55+
return self
56+
57+
def __next__(self):
58+
if self.closed:
59+
raise StopIteration
60+
return next(self.logs)
61+
62+
def close(self):
63+
self.closed = True
64+
65+
mock_logs = MockLogGenerator([b'test log 1\n', b'test log 2\n'])
66+
mock_container.logs.return_value = mock_logs
67+
68+
streamer = LogStreamer(mock_container, mock_log_fn)
69+
assert streamer.stdout_thread is not None
70+
assert streamer.log_generator is not None
71+
72+
streamer.close()
73+
74+
# Verify logs were processed
75+
expected_calls = [
76+
('debug', '[inside container] test log 1'),
77+
('debug', '[inside container] test log 2'),
78+
]
79+
actual_calls = [(args[0], args[1]) for args, _ in mock_log_fn.call_args_list]
80+
for expected in expected_calls:
81+
assert expected in actual_calls
82+
83+
84+
def test_del_without_thread(mock_container, mock_log_fn):
85+
"""Test that __del__ works even if stdout_thread was not initialized."""
86+
streamer = LogStreamer(mock_container, mock_log_fn)
87+
delattr(
88+
streamer, 'stdout_thread'
89+
) # Simulate case where the thread was never created
90+
streamer.__del__() # Should not raise any exceptions

0 commit comments

Comments
 (0)