Uh oh!
There was an error while loading. Please reload this page.
gh-142654: show the clear error message when sampling on an unknown PID - #142655
Conversation
…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
RemoteUnwinderinitialization in a try-except block that converts exceptions toSystemExit - Updates test expectations from
OSError/RuntimeErrortoSystemExit - 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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingy94@gmail.com>
pablogsal
commented
Dec 14, 2025
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingy94@gmail.com>
kemingy
commented
Dec 15, 2025
Hi @pablogsal I have added the process check. Please take a look. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| 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__?
Uh oh!
There was an error while loading. Please reload this page.
pablogsal
commented
Dec 15, 2025
We also need a test for this |
Signed-off-by: Keming <kemingy94@gmail.com>
kemingy
commented
Dec 16, 2025
Hi @pablogsal Thanks for your advice. I have added the |
Signed-off-by: Keming <kemingy94@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.
pablogsal
commented
Dec 17, 2025
Made some small fixes but great job @kemingy! Thanks for the PR 🚀 |
Before this change:
After this change:
This aligns the behavior with attaching to a process that doesn't have the PyRuntime section.