-
Notifications
You must be signed in to change notification settings - Fork 903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cli: Catch panics to tell the user to report them #1928
cli: Catch panics to tell the user to report them #1928
Conversation
909eb1e
to
3c15d76
Compare
Were you running into a panic somewhere specific? |
No this is just a precaution seeing issues like #1845. Users might not know what to do when they ruff panics, so I think it makes sense to ask them to report panics to us when they occur. Since I think we should treat any panic that occurs during normal operation as a bug. |
ruff_cli/src/main.rs
Outdated
@@ -91,6 +91,12 @@ fn resolve( | |||
pub fn main() -> Result<ExitCode> { | |||
// Extract command-line arguments. | |||
let (cli, overrides) = Cli::parse().partition(); | |||
|
|||
drop(std::panic::catch_unwind(|| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant to use std::panic::set_hook
? Maybe also tell user to report the backtrace by re-run with RUST_BACKTRACE=1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to use catch_unwind
because it does not replace the default panic output it just gets invoked before it so the regular output telling the user to re-run with RUST_BACKTRACE=1 is still printed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this currently always print the message?
cargo run -- --no-cache scripts/add_plugin.py
Finished dev [unoptimized + debuginfo] target(s) in 0.11s
Running `target/debug/ruff --no-cache scripts/add_plugin.py`
ruff crashed ... this should not be happening :/
Please report it on GitHub: https://github.com/charliermarsh/ruff/issues/new?title=Panic+when+%2E%2E%2E
scripts/add_plugin.py:105:89: E501 Line too long (111 > 88 characters)
Found 1 error(s).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, thanks! Good catch :) I have updated it to use set_hook
as you suggested and now it only prints when it actually panics. I am still invoking the default hook for the RUST_BACKTRACE=1 message.
3c15d76
to
d158085
Compare
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) | `^0.0.224` -> `^0.0.225` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.225/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.225/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.225/compatibility-slim/0.0.224)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.225/confidence-slim/0.0.224)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.225`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.225) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.224...v0.0.225) #### What's Changed - Define origin names & URLs within doc comments by [@​not-my-profile](https://togithub.com/not-my-profile) in [astral-sh/ruff#1929 - Refactor settings by [@​not-my-profile](https://togithub.com/not-my-profile) in [astral-sh/ruff#1930 - Allow duplicate enum values for enum.auto() by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1933 - Move `@functools.cache` rewrites to their own rule by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1938 - cli: Catch panics to tell the user to report them by [@​not-my-profile](https://togithub.com/not-my-profile) in [astral-sh/ruff#1928 - Avoid autofixing comma rules when --fix is not set by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1940 - Avoid broken autofix for `SIM103` with `elif` by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1944 - Implement `flake8-no-pep420` by [@​edgarrmondragon](https://togithub.com/edgarrmondragon) in [astral-sh/ruff#1942 - Exempt `contextlib.ExitStack()` for SIM115 rules by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1946 - Restrict SIM105 to try blocks with a body of one simple statement by [@​andersk](https://togithub.com/andersk) in [astral-sh/ruff#1948 **Full Changelog**: astral-sh/ruff@v0.0.224...v0.0.225 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDUuMyIsInVwZGF0ZWRJblZlciI6IjM0LjEwNS4zIn0=--> Signed-off-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
No description provided.