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

False positive for try-except-raise with multiple exceptions catching #8051

Closed
fly opened this issue Jan 12, 2023 · 3 comments · Fixed by #8052
Closed

False positive for try-except-raise with multiple exceptions catching #8051

fly opened this issue Jan 12, 2023 · 3 comments · Fixed by #8052
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@fly
Copy link
Contributor

fly commented Jan 12, 2023

Bug description

Error is produced when a single except statement catches several exceptions for re-raising and when these exceptions are not in the current namespace.

Example (both Exception1 and Exception2 here are inherited directly from Exception):

from app import errors

def check():
    try:
        return do_something()
    except (errors.Exception1, errors.Exception2):  # The except handler raises immediately (try-except-raise)
        raise
    except Exception as e:
        raise errors.Exception1("Boom!") from e

Note that if I bring exceptions directly into namespace, then it works:

from app.errors import Exception1, Exception2

def check():
    try:
        return do_something()
    except (Exception1, Exception2): 
        raise
    except Exception as e:
        raise Exception1("Boom!") from e

It also works if I use separate except statements for each:

from app import errors

def check():
    try:
        return do_something()
    except errors.Exception1:
        raise
    except errors.Exception2:
        raise
    except Exception as e:
        raise errors.Exception1("Boom!") from e

Configuration

No response

Command used

pylint check.py

Pylint output

************* Module check
check.py:19:4: W0706: The except handler raises immediately (try-except-raise)

Expected behavior

No errors

Pylint version

pylint 2.15.10
astroid 2.13.2
Python 3.9.16 (main, Dec  7 2022, 01:12:08) 
[GCC 11.3.0]

OS / Environment

No response

Additional dependencies

No response

@fly fly added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 12, 2023
@Pierre-Sassoulas
Copy link
Member

Hello, thank you for opening the issue. What is the inheritance tree of errors.Exception1, errors.Exception2 and Exception ?

@Pierre-Sassoulas Pierre-Sassoulas added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 12, 2023
@fly
Copy link
Contributor Author

fly commented Jan 12, 2023

both Exception1 and Exception2 here are inherited directly from Exception. Exception is built in

@Pierre-Sassoulas Pierre-Sassoulas added Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels Jan 12, 2023
@Pierre-Sassoulas
Copy link
Member

It's indeed a false positive then. We probably need to use inference to handle exception's aliases.

@Pierre-Sassoulas Pierre-Sassoulas added the Good first issue Friendly and approachable by new contributors label Jan 12, 2023
Pierre-Sassoulas pushed a commit that referenced this issue Jan 12, 2023
…in one except statement if exception are in different namespace (#8052)

Closes #8051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants