Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions python/packages/jumpstarter/jumpstarter/exporter/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,11 @@ async def read_pty_output() -> None: # noqa: C901
# PTY closed or read error
logger.debug("read_pty_output: OSError in loop: %s", e)
break

except Exception as e:
logger.debug("read_pty_output: unexpected error in loop: %s", e)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we prefer to surface the error anyway in case it glitched? maybe worth changing to l logger.warning with exc_info=True?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure, it's about expected edge case failures (like OSError above) so raising level would generate noise

break

finally:
# Drain any remaining data from the PTY buffer.
# On macOS, PTY output may still be in the kernel buffer
Expand Down
27 changes: 27 additions & 0 deletions python/packages/jumpstarter/jumpstarter/exporter/hooks_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,33 @@ def flush_lines_with_drain_error(buffer, output_lines):
result = await executor.execute_before_lease_hook(lease_scope)
assert result is None

@macos_pty_xfail
async def test_main_loop_non_oserror_is_caught(self, lease_scope) -> None:
"""Verify that a non-OSError exception in the main read loop is caught
by the except-Exception handler and does not propagate to the caller.
"""
hook_config = HookConfigV1Alpha1(
before_lease=HookInstanceConfigV1Alpha1(
script="echo MAIN_LOOP_ERROR",
timeout=10,
),
)
executor = HookExecutor(config=hook_config)

def flush_lines_always_error(buffer, output_lines):
raise ValueError("simulated non-OSError")

with (
patch("jumpstarter.exporter.hooks._flush_lines", side_effect=flush_lines_always_error),
patch("jumpstarter.exporter.hooks.logger") as mock_logger,
):
result = await executor.execute_before_lease_hook(lease_scope)
assert result is None
debug_calls = [str(c) for c in mock_logger.debug.call_args_list]
assert any("unexpected error in loop" in c for c in debug_calls), (
f"Expected main-loop exception handler to log, got: {debug_calls}"
)

@macos_pty_xfail
async def test_drain_retries_empty_select_then_captures_data(self, lease_scope) -> None:
"""Verify that the drain retries after empty select() calls and still
Expand Down
Loading