Skip to content

Commit 9d92ac1

Browse files
authored
gh-143040: Exit taychon live mode gracefully and display profiled script errors (#143101)
1 parent a1c6308 commit 9d92ac1

File tree

6 files changed

+133
-38
lines changed

6 files changed

+133
-38
lines changed

Lib/profiling/sampling/_sync_coordinator.py

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def _execute_module(module_name: str, module_args: List[str]) -> None:
135135
module_args: Arguments to pass to the module
136136
137137
Raises:
138-
TargetError: If module execution fails
138+
TargetError: If module cannot be found
139139
"""
140140
# Replace sys.argv to match how Python normally runs modules
141141
# When running 'python -m module args', sys.argv is ["__main__.py", "args"]
@@ -145,11 +145,8 @@ def _execute_module(module_name: str, module_args: List[str]) -> None:
145145
runpy.run_module(module_name, run_name="__main__", alter_sys=True)
146146
except ImportError as e:
147147
raise TargetError(f"Module '{module_name}' not found: {e}") from e
148-
except SystemExit:
149-
# SystemExit is normal for modules
150-
pass
151-
except Exception as e:
152-
raise TargetError(f"Error executing module '{module_name}': {e}") from e
148+
# Let other exceptions (including SystemExit) propagate naturally
149+
# so Python prints the full traceback to stderr
153150

154151

155152
def _execute_script(script_path: str, script_args: List[str], cwd: str) -> None:
@@ -183,22 +180,20 @@ def _execute_script(script_path: str, script_args: List[str], cwd: str) -> None:
183180
except PermissionError as e:
184181
raise TargetError(f"Permission denied reading script: {script_path}") from e
185182

186-
try:
187-
main_module = types.ModuleType("__main__")
188-
main_module.__file__ = script_path
189-
main_module.__builtins__ = __builtins__
190-
# gh-140729: Create a __mp_main__ module to allow pickling
191-
sys.modules['__main__'] = sys.modules['__mp_main__'] = main_module
183+
main_module = types.ModuleType("__main__")
184+
main_module.__file__ = script_path
185+
main_module.__builtins__ = __builtins__
186+
# gh-140729: Create a __mp_main__ module to allow pickling
187+
sys.modules['__main__'] = sys.modules['__mp_main__'] = main_module
192188

189+
try:
193190
code = compile(source_code, script_path, 'exec', module='__main__')
194-
exec(code, main_module.__dict__)
195191
except SyntaxError as e:
196192
raise TargetError(f"Syntax error in script {script_path}: {e}") from e
197-
except SystemExit:
198-
# SystemExit is normal for scripts
199-
pass
200-
except Exception as e:
201-
raise TargetError(f"Error executing script '{script_path}': {e}") from e
193+
194+
# Execute the script - let exceptions propagate naturally so Python
195+
# prints the full traceback to stderr
196+
exec(code, main_module.__dict__)
202197

203198

204199
def main() -> NoReturn:
@@ -209,6 +204,8 @@ def main() -> NoReturn:
209204
with the sample profiler by signaling when the process is ready
210205
to be profiled.
211206
"""
207+
# Phase 1: Parse arguments and set up environment
208+
# Errors here are coordinator errors, not script errors
212209
try:
213210
# Parse and validate arguments
214211
sync_port, cwd, target_args = _validate_arguments(sys.argv)
@@ -237,21 +234,19 @@ def main() -> NoReturn:
237234
# Signal readiness to profiler
238235
_signal_readiness(sync_port)
239236

240-
# Execute the target
241-
if is_module:
242-
_execute_module(module_name, module_args)
243-
else:
244-
_execute_script(script_path, script_args, cwd)
245-
246237
except CoordinatorError as e:
247238
print(f"Profiler coordinator error: {e}", file=sys.stderr)
248239
sys.exit(1)
249240
except KeyboardInterrupt:
250241
print("Interrupted", file=sys.stderr)
251242
sys.exit(1)
252-
except Exception as e:
253-
print(f"Unexpected error in profiler coordinator: {e}", file=sys.stderr)
254-
sys.exit(1)
243+
244+
# Phase 2: Execute the target script/module
245+
# Let exceptions propagate naturally so Python prints full tracebacks
246+
if is_module:
247+
_execute_module(module_name, module_args)
248+
else:
249+
_execute_script(script_path, script_args, cwd)
255250

256251
# Normal exit
257252
sys.exit(0)

Lib/profiling/sampling/cli.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,11 +272,6 @@ def _run_with_sync(original_cmd, suppress_output=False):
272272

273273
try:
274274
_wait_for_ready_signal(sync_sock, process, _SYNC_TIMEOUT_SEC)
275-
276-
# Close stderr pipe if we were capturing it
277-
if process.stderr:
278-
process.stderr.close()
279-
280275
except socket.timeout:
281276
# If we timeout, kill the process and raise an error
282277
if process.poll() is None:
@@ -1103,14 +1098,27 @@ def _handle_live_run(args):
11031098
blocking=args.blocking,
11041099
)
11051100
finally:
1106-
# Clean up the subprocess
1107-
if process.poll() is None:
1101+
# Clean up the subprocess and get any error output
1102+
returncode = process.poll()
1103+
if returncode is None:
1104+
# Process still running - terminate it
11081105
process.terminate()
11091106
try:
11101107
process.wait(timeout=_PROCESS_KILL_TIMEOUT_SEC)
11111108
except subprocess.TimeoutExpired:
11121109
process.kill()
1113-
process.wait()
1110+
# Ensure process is fully terminated
1111+
process.wait()
1112+
# Read any stderr output (tracebacks, errors, etc.)
1113+
if process.stderr:
1114+
with process.stderr:
1115+
try:
1116+
stderr = process.stderr.read()
1117+
if stderr:
1118+
print(stderr.decode(), file=sys.stderr)
1119+
except (OSError, ValueError):
1120+
# Ignore errors if pipe is already closed
1121+
pass
11141122

11151123

11161124
def _handle_replay(args):

Lib/profiling/sampling/live_collector/collector.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,9 @@ def __init__(
216216
def elapsed_time(self):
217217
"""Get the elapsed time, frozen when finished."""
218218
if self.finished and self.finish_timestamp is not None:
219+
# Handle case where process exited before any samples were collected
220+
if self.start_time is None:
221+
return 0
219222
return self.finish_timestamp - self.start_time
220223
return time.perf_counter() - self.start_time if self.start_time else 0
221224

Lib/profiling/sampling/sample.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ def _pause_threads(unwinder, blocking):
4242
LiveStatsCollector = None
4343

4444
_FREE_THREADED_BUILD = sysconfig.get_config_var("Py_GIL_DISABLED") is not None
45-
45+
# Minimum number of samples required before showing the TUI
46+
# If fewer samples are collected, we skip the TUI and just print a message
47+
MIN_SAMPLES_FOR_TUI = 200
4648

4749
class SampleProfiler:
4850
def __init__(self, pid, sample_interval_usec, all_threads, *, mode=PROFILING_MODE_WALL, native=False, gc=True, opcodes=False, skip_non_matching_threads=True, collect_stats=False, blocking=False):
@@ -459,6 +461,11 @@ def sample_live(
459461
"""
460462
import curses
461463

464+
# Check if process is alive before doing any heavy initialization
465+
if not _is_process_running(pid):
466+
print(f"No samples collected - process {pid} exited before profiling could begin.", file=sys.stderr)
467+
return collector
468+
462469
# Get sample interval from collector
463470
sample_interval_usec = collector.sample_interval_usec
464471

@@ -486,6 +493,12 @@ def curses_wrapper_func(stdscr):
486493
collector.init_curses(stdscr)
487494
try:
488495
profiler.sample(collector, duration_sec, async_aware=async_aware)
496+
# If too few samples were collected, exit cleanly without showing TUI
497+
if collector.successful_samples < MIN_SAMPLES_FOR_TUI:
498+
# Clear screen before exiting to avoid visual artifacts
499+
stdscr.clear()
500+
stdscr.refresh()
501+
return
489502
# Mark as finished and keep the TUI running until user presses 'q'
490503
collector.mark_finished()
491504
# Keep processing input until user quits
@@ -500,4 +513,11 @@ def curses_wrapper_func(stdscr):
500513
except KeyboardInterrupt:
501514
pass
502515

516+
# If too few samples were collected, print a message
517+
if collector.successful_samples < MIN_SAMPLES_FOR_TUI:
518+
if collector.successful_samples == 0:
519+
print(f"No samples collected - process {pid} exited before profiling could begin.", file=sys.stderr)
520+
else:
521+
print(f"Only {collector.successful_samples} sample(s) collected (minimum {MIN_SAMPLES_FOR_TUI} required for TUI) - process {pid} exited too quickly.", file=sys.stderr)
522+
503523
return collector

Lib/test/test_profiling/test_sampling_profiler/test_cli.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
from profiling.sampling.cli import main
1919
from profiling.sampling.errors import SamplingScriptNotFoundError, SamplingModuleNotFoundError, SamplingUnknownProcessError
2020

21-
2221
class TestSampleProfilerCLI(unittest.TestCase):
2322
def _setup_sync_mocks(self, mock_socket, mock_popen):
2423
"""Helper to set up socket and process mocks for coordinator tests."""

Lib/test/test_profiling/test_sampling_profiler/test_live_collector_ui.py

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,27 @@
44
edge cases, update display, and display helpers.
55
"""
66

7+
import functools
8+
import io
79
import sys
10+
import tempfile
811
import time
912
import unittest
1013
from unittest import mock
11-
from test.support import requires
14+
from test.support import requires, requires_remote_subprocess_debugging
1215
from test.support.import_helper import import_module
1316

1417
# Only run these tests if curses is available
1518
requires("curses")
1619
curses = import_module("curses")
1720

1821
from profiling.sampling.live_collector import LiveStatsCollector, MockDisplay
22+
from profiling.sampling.cli import main
1923
from ._live_collector_helpers import (
2024
MockThreadInfo,
2125
MockInterpreterInfo,
2226
)
27+
from .helpers import close_and_unlink
2328

2429

2530
class TestLiveStatsCollectorWithMockDisplay(unittest.TestCase):
@@ -816,5 +821,70 @@ def test_get_all_lines_full_display(self):
816821
self.assertTrue(any("PID" in line for line in lines))
817822

818823

824+
@requires_remote_subprocess_debugging()
825+
class TestLiveModeErrors(unittest.TestCase):
826+
"""Tests running error commands in the live mode fails gracefully."""
827+
828+
def mock_curses_wrapper(self, func):
829+
func(mock.MagicMock())
830+
831+
def mock_init_curses_side_effect(self, n_times, mock_self, stdscr):
832+
mock_self.display = MockDisplay()
833+
# Allow the loop to run for a bit (approx 0.5s) before quitting
834+
# This ensures we don't exit too early while the subprocess is
835+
# still failing
836+
for _ in range(n_times):
837+
mock_self.display.simulate_input(-1)
838+
if n_times >= 500:
839+
mock_self.display.simulate_input(ord('q'))
840+
841+
def test_run_failed_module_live(self):
842+
"""Test that running a existing module that fails exits with clean error."""
843+
844+
args = [
845+
"profiling.sampling.cli", "run", "--live", "-m", "test",
846+
"test_asdasd"
847+
]
848+
849+
with (
850+
mock.patch(
851+
'profiling.sampling.live_collector.collector.LiveStatsCollector.init_curses',
852+
autospec=True,
853+
side_effect=functools.partial(self.mock_init_curses_side_effect, 1000)
854+
),
855+
mock.patch('curses.wrapper', side_effect=self.mock_curses_wrapper),
856+
mock.patch("sys.argv", args),
857+
mock.patch('sys.stderr', new=io.StringIO()) as fake_stderr
858+
):
859+
main()
860+
self.assertIn(
861+
'test test_asdasd crashed -- Traceback (most recent call last):',
862+
fake_stderr.getvalue()
863+
)
864+
865+
def test_run_failed_script_live(self):
866+
"""Test that running a failing script exits with clean error."""
867+
script = tempfile.NamedTemporaryFile(suffix=".py")
868+
self.addCleanup(close_and_unlink, script)
869+
script.write(b'1/0\n')
870+
script.seek(0)
871+
872+
args = ["profiling.sampling.cli", "run", "--live", script.name]
873+
874+
with (
875+
mock.patch(
876+
'profiling.sampling.live_collector.collector.LiveStatsCollector.init_curses',
877+
autospec=True,
878+
side_effect=functools.partial(self.mock_init_curses_side_effect, 200)
879+
),
880+
mock.patch('curses.wrapper', side_effect=self.mock_curses_wrapper),
881+
mock.patch("sys.argv", args),
882+
mock.patch('sys.stderr', new=io.StringIO()) as fake_stderr
883+
):
884+
main()
885+
stderr = fake_stderr.getvalue()
886+
self.assertIn('ZeroDivisionError', stderr)
887+
888+
819889
if __name__ == "__main__":
820890
unittest.main()

0 commit comments

Comments
 (0)