gh-142654: show the clear error message when sampling on an unknown PID - #142655
gh-142654: show the clear error message when sampling on an unknown PID#142655pablogsal merged 10 commits into
Conversation
Sorry, something went wrong.
…nown PID Signed-off-by: Keming <kemingy94@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR improves error handling when attempting to profile a process with an invalid PID by simplifying the error message. Instead of showing a confusing stack trace with multiple exceptions, it now displays only the final, clear error message about failing to find the PyRuntime section.
Key Changes:
- Wraps RemoteUnwinder initialization in a try-except block that converts exceptions to SystemExit
- Updates test expectations from OSError/RuntimeError to SystemExit
- Adds a NEWS entry documenting the improvement
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Lib/profiling/sampling/sample.py | Adds exception handling in SampleProfiler.__init__ to convert errors to SystemExit for cleaner user-facing messages |
| Lib/test/test_profiling/test_sampling_profiler/test_integration.py | Updates test to expect SystemExit instead of OSError/RuntimeError for invalid PIDs |
| Misc/NEWS.d/next/Library/2025-12-13-10-34-59.gh-issue-142654.fmm974.rst | Documents the error message improvement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingy94@gmail.com>
|
@kemingy can you approach this similarly as #142592 ? The idea is to not show an ugly stack trace or difficult to understand messages like "Failed to find the PyRuntime section in process -1 on Linux platform" but just checking if the process exists or not or if is valid or not |
Sorry, something went wrong.
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingy94@gmail.com>
Hi @pablogsal I have added the process check. Please take a look. |
Sorry, something went wrong.
| def _handle_attach(args): | ||
| """Handle the 'attach' command.""" | ||
| if not _is_process_running(args.pid): | ||
| raise sys.exit(f"Process with PID {args.pid} is not running.") |
There was a problem hiding this comment.
I would prefer to raise some custom exception here and catch it in __main__ like the other exceptions. Perhaps we should also do the same for the other usages of sys.exit.
Do you mind creating a errors.py file with custom exceptions for the different case, raise them and properly handle the messages in __main__?
Sorry, something went wrong.
|
We also need a test for this |
Sorry, something went wrong.
Signed-off-by: Keming <kemingy94@gmail.com>
Hi @pablogsal Thanks for your advice. I have added the errors.py file and handle these custom exceptions in the __main__. Also added a test for the attach cli. Please take a look. |
Sorry, something went wrong.
Signed-off-by: Keming <kemingy94@gmail.com>
|
Made some small fixes but great job @kemingy! Thanks for the PR 🚀 |
Sorry, something went wrong.
Before this change:
OSError: Cannot open process memory map file '/proc/-1/maps' for PID -1 section search: No such file or directory During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/keming/GitHub/cpython/Lib/runpy.py", line 196, in _run_module_as_main return _run_code(code, main_globals, None, "__main__", mod_spec) File "/home/keming/GitHub/cpython/Lib/runpy.py", line 87, in _run_code exec(code, run_globals) ~~~~^^^^^^^^^^^^^^^^^^^ File "/home/keming/GitHub/cpython/Lib/profiling/sampling/__main__.py", line 64, in <module> main() ~~~~^^ File "/home/keming/GitHub/cpython/Lib/profiling/sampling/cli.py", line 592, in main handler(args) ~~~~~~~^^^^^^ File "/home/keming/GitHub/cpython/Lib/profiling/sampling/cli.py", line 620, in _handle_attach collector = sample( args.pid, ...<8 lines>... opcodes=args.opcodes, ) File "/home/keming/GitHub/cpython/Lib/profiling/sampling/sample.py", line 327, in sample profiler = SampleProfiler( pid, ...<7 lines>... collect_stats=realtime_stats, ) File "/home/keming/GitHub/cpython/Lib/profiling/sampling/sample.py", line 39, in __init__ self.unwinder = _remote_debugging.RemoteUnwinder( ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ self.pid, all_threads=self.all_threads, mode=mode, native=native, gc=gc, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ opcodes=opcodes, skip_non_matching_threads=skip_non_matching_threads, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cache_frames=True, stats=collect_stats ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ) ^ RuntimeError: Failed to find the PyRuntime section in process -1 on Linux platformAfter this change:
This aligns the behavior with attaching to a process that doesn't have the PyRuntime section.