Skip to content
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

Suggestion to use bare raise instead of raise exc #4333

Closed
zanieb opened this issue May 9, 2023 · 4 comments
Closed

Suggestion to use bare raise instead of raise exc #4333

zanieb opened this issue May 9, 2023 · 4 comments
Labels
question Asking for support or clarification

Comments

@zanieb
Copy link
Member

zanieb commented May 9, 2023

I've been championing the use of raise instead of raise exc when re-raising a captured exception as it removes a frame from the traceback. When many exceptions are chained, this can make a significant difference in the length of a traceback.

WRONG: Extra frame included in traceback

try:
    raise ValueError("1")
except Exception as exc:
    raise exc
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 4, in <module>
    raise exc
  File "/Users/mz/dev/prefect/example.py", line 2, in <module>
    raise ValueError("1")
ValueError: 1

RIGHT: No frame added when the exception is re-raised

try:
    raise ValueError("1")
except Exception:
    raise
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 2, in <module>
    raise ValueError("1")
ValueError: 1

I don't think this is covered by any existing linters. I checked flake8 and pylint.

@tjkuson
Copy link
Contributor

tjkuson commented May 9, 2023

As I understand it, this is covered by tryceratops and is implemented in ruff as TRY201.

@charliermarsh charliermarsh added the question Asking for support or clarification label May 10, 2023
@charliermarsh
Copy link
Member

Thanks for the clear issue :) My read is that this is covered by TRY201 as @tjkuson mentioned -- but just LMK if I'm misunderstanding.

@zanieb
Copy link
Member Author

zanieb commented May 10, 2023

Cool! I'm interested in adding explanation there as they call it "redundant" but it has an actual runtime effect.

What kind of criteria is there for a rule being included by default?

I've noticed this is also not an auto-fixable issue but it seems like it could be

example.py:4:11: TRY201 Use `raise` without specifying exception name
Found 1 error.

@tylerlaprade
Copy link
Contributor

Hi @zanieb and @charliermarsh, I would also enjoy seeing this rule become auto-fixable.

charliermarsh pushed a commit that referenced this issue Jul 24, 2023
## Summary

Make `TRY201` always autofiable.

## Test Plan

1. `cargo test`
2. `cargo insta review`

ref:
#4333 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Asking for support or clarification
Projects
None yet
Development

No branches or pull requests

4 participants