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

runpy swallows ImportError information with relative imports #59521

Closed
cjerdonek opened this issue Jul 10, 2012 · 14 comments
Closed

runpy swallows ImportError information with relative imports #59521

cjerdonek opened this issue Jul 10, 2012 · 14 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@cjerdonek
Copy link
Member

BPO 15316
Nosy @brettcannon, @birkenfeld, @amauryfa, @ncoghlan, @bitdancer, @cjerdonek, @ericsnowcurrently
Files
  • issue-15316-failing-test-1.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/brettcannon'
    closed_at = <Date 2012-08-24.22:27:06.079>
    created_at = <Date 2012-07-10.05:39:17.846>
    labels = ['type-bug', 'library']
    title = 'runpy swallows ImportError information with relative imports'
    updated_at = <Date 2012-08-24.22:27:06.079>
    user = 'https://github.com/cjerdonek'

    bugs.python.org fields:

    activity = <Date 2012-08-24.22:27:06.079>
    actor = 'brett.cannon'
    assignee = 'brett.cannon'
    closed = True
    closed_date = <Date 2012-08-24.22:27:06.079>
    closer = 'brett.cannon'
    components = ['Library (Lib)']
    creation = <Date 2012-07-10.05:39:17.846>
    creator = 'chris.jerdonek'
    dependencies = []
    files = ['26967']
    hgrepos = []
    issue_num = 15316
    keywords = ['patch', '3.2regression']
    message_count = 14.0
    messages = ['165166', '165175', '168830', '168831', '168846', '168896', '168903', '168904', '168905', '168907', '169031', '169036', '169093', '169096']
    nosy_count = 9.0
    nosy_names = ['brett.cannon', 'georg.brandl', 'amaury.forgeotdarc', 'ncoghlan', 'r.david.murray', 'chris.jerdonek', 'python-dev', 'eric.snow', 'jeffknupp']
    pr_nums = []
    priority = 'critical'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue15316'
    versions = ['Python 3.3']

    @cjerdonek
    Copy link
    Member Author

    With the following package directory structure--

    foo/
    __init__.py
    __main__.py
    from foo import bar
    bar.py
    print('***')
    raise ImportError('test...')

    Running--

    $ ./python.exe -m foo

    Yields--

    ***
    Traceback (most recent call last):
      File ".../python/cpython/cpython/Lib/runpy.py", line 162, in _run_module_as_main
        "__main__", fname, loader, pkg_name)
      File ".../python/cpython/cpython/Lib/runpy.py", line 75, in _run_code
        exec(code, run_globals)
      File "./foo/__main__.py", line 1, in <module>
        from foo import bar
    ImportError: cannot import name bar
    $

    The exception text gets swallowed.

    Changing the relative import "from foo import bar" to "import foo.bar" does show the exception text.

    @cjerdonek cjerdonek added the stdlib Python modules in the Lib dir label Jul 10, 2012
    @amauryfa
    Copy link
    Member

    Looks very similar to bpo-15111.

    @cjerdonek
    Copy link
    Member Author

    I randomly ran into this issue again. I'm not sure this was ever resolved (i.e. I think it may always have been different from bpo-15111).

    I still get the above behavior in the default branch.

    And here is what I get in the 3.2 branch (the error information is not swallowed):
     
    $ ./python.exe -m foo
    ***
    Traceback (most recent call last):
      File ".../Lib/runpy.py", line 161, in _run_module_as_main
        "__main__", fname, loader, pkg_name)
      File ".../Lib/runpy.py", line 74, in _run_code
        exec(code, run_globals)
      File ".../foo/__main__.py", line 1, in <module>
        from foo import bar
      File "foo/bar.py", line 2, in <module>
        raise ImportError('test...')
    ImportError: test...

    @cjerdonek cjerdonek reopened this Aug 22, 2012
    @cjerdonek cjerdonek added the type-bug An unexpected behavior, bug, or error label Aug 22, 2012
    @cjerdonek
    Copy link
    Member Author

    I ran into this again because an error while running ./python.exe -m test was getting masked. The use of main.py in the package may be the distinguishing characteristic.

    @cjerdonek
    Copy link
    Member Author

    Should this issue be fixed before the release? If it is not fixed, certain problems found after the release may become harder to report and diagnose (because the true source of error will be masked).

    Two months ago bpo-15111 which was thought to be the same was given a priority of "high".

    @brettcannon
    Copy link
    Member

    It has nothing to do with runpy and __main__.py and everything to do with rev 78619:0d52f125dd32 (which fixed issue bpo-15715) which was done to ignore bogus names in fromlist. If you simply change the import line to "import foo.bar" then you get the expected result.

    The options I see to solve this without invalidating issue bpo-15715 is:

    1. Parse the ImportError message to notice that it was a lookup failure and not some other ImportError (ewww)

    2. Importlib has to tag every exception is raises because of a finder failure to know when an ImportError was triggered because the module wasn't found (less eww, but still brittle as it means only importlib or people aware of the flag will have the expected semantics)

    3. Create a ModuleNotFoundError exception that subclasses ImportError and catch that (breaks doctests and introduces a new exception that people will need to be aware of, plus the question of whether it should just exist in importlib or be a builtin)

    4. change importlib._bootstrap._handle_fromlist (and various other bits of importlib code) to call _find_module() and silently ignore when no finder is found as desired (probably need to add a flag to _find_and_load() to signal whether finder failure just returns None or raises ImportError)

    While I prefer 3, I think it's a bit late in the release to try to introduce a new exception to begin separating the meaning of 16 different raise ImportError cases in importlib._bootstrap based on inheritance. My gut says 4 is the best solution given the timeframe. I should be able to get a patch in on Friday if that works for people.

    @cjerdonek
    Copy link
    Member Author

    Here is a formal unit test case that passes in 3.2 but not in 3.3 (a "simpler" case not using __main__.py).

    (script_helper.create_empty_file() doesn't seem to be available in 3.2.)

    @brettcannon
    Copy link
    Member

    Thanks for the test, Chris. It can probably be simplified using the utilities in test_importlib (e.g. managing the cleanup of sys.path, using mocked loaders to raise the exception instead of having to write to the file system, etc.), but otherwise the idea of the test is accurate.

    @cjerdonek
    Copy link
    Member Author

    You're welcome, Brett. I'll let you or someone else recast the test using the latest preferred techniques. I was just using the style of the immediately surrounding tests.

    @ericsnowcurrently
    Copy link
    Member

    While I prefer 3, I think it's a bit late in the release to try to
    introduce a new exception to begin separating the meaning of 16
    different raise ImportError cases in importlib._bootstrap based on
    inheritance. My gut says 4 is the best solution given the timeframe.

    Agreed. I still think option 3 would be suitable and have created bpo-15767 to track it.

    @birkenfeld
    Copy link
    Member

    I don't agree that this is a blocker; would be nice to fix it, of course.

    @brettcannon
    Copy link
    Member

    It will get fixed today.

    @brettcannon
    Copy link
    Member

    I am running the test suite now using the "secret" attribute on ImportError. I tried to pass a flag, but locking became a bit messy/complicated. And I also realized that if I didn't do this then using different implementation of import_ in importlib wouldn't work because I would be hard-coding in what import implementation was used which would bypass the accelerated code in import.c. Plus with the compatibility issues I have had in the passed, I didn't want to skip a step of import that someone was probably relying on.

    Once Python 3.4 comes out I will create a new exception that is raised when no module is found and catch that class specifically to avoid this hack.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 24, 2012

    New changeset ca4bf8e10bc0 by Brett Cannon in branch 'default':
    Issue bpo-15316: Let exceptions raised during imports triggered by the
    http://hg.python.org/cpython/rev/ca4bf8e10bc0

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants