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

User Uncaught Exceptions setting should be false by default #751

Closed
RobertCsordas opened this issue Oct 12, 2021 · 12 comments
Closed

User Uncaught Exceptions setting should be false by default #751

RobertCsordas opened this issue Oct 12, 2021 · 12 comments
Assignees

Comments

@RobertCsordas
Copy link

Environment data

  • VS Code version: 1.61.0
  • Extension version (available under the Extensions sidebar): v2021.10.1317843341
  • OS and version: Ubuntu 20.04.2 LTS
  • Python version (& distribution if applicable, e.g. Anaconda): 3.8.10
  • Type of virtual environment used (N/A | venv | virtualenv | conda | ...): N/A
  • Relevant/affected Python packages and their versions: pytorch (troch==1.9.0)
  • Relevant/affected Python-related VS Code extensions and their versions: XXX
  • Value of the python.languageServer setting: Pylance

Expected behaviour

The following code should not raise an exception.

import torch.utils.data


class Cls():
    def __getattr__(self, item):
        raise AttributeError()


o = Cls()
print(isinstance(o, torch.utils.data.IterableDataset))

Actual behaviour

The code stops on the "raise AttributeError()" line and shows an exception is raised. If one clicks "Continue", the code continues correctly. But it is still pretty annoying to click it every single time it happens in a big project.

If one runs it from the terminal without VSCode debugger, it runs without issues.

Steps to reproduce:

Run the code above from vscode. The launch.json entry for it:

{
    "type": "python",
    "request": "launch",
    "name": "Debug File",
    "justMyCode": false,
    "program": "${file}",
    "cwd": "${fileDirname}"
}

Other

It might be related to microsoft/vscode-python#17687, but not sure.

@karthiknadig karthiknadig transferred this issue from microsoft/vscode-python Oct 12, 2021
@sknowles0143
Copy link

We are seeing similar issues with ImportErrors stopping our flask server from starting only when run through VSC. This same code is running in our development environment with no errors and in docker containers locally. If you hit play on each exception (12 to 20 times) you can eventually step through all the exceptions and the server will eventually start.

@fabioz
Copy link
Collaborator

fabioz commented Oct 12, 2021

There's a new exception setting (which by default is true) which tracks User Uncaught Exceptions.

This setting is close to the Raised Exceptions in the sense that it will trigger whenever an exception raised, but it'll be shown only if the user code reaches library code (which is what is happening here).

It's very useful when users want to deal with exceptions from test frameworks (for instance, an exception raised in a test will be shown if it bubbles up in the test framework), but in this particular case, I agree it's really not useful.

The solution is unchecking the User Uncaught Exceptions setting (under Breakpoints).

@karthiknadig @int19h Do you think this should be set to false as the default as it seems some users can be confused by it? Although maybe this is just a matter of having users aware of it too...

@fabioz fabioz changed the title Python debugger stops on exceptions which is handled internally and should not be visible Should User Uncaught Exceptions setting be false by default? Oct 12, 2021
@sknowles0143
Copy link

These are not Uncaught User Exceptions. These Exceptions only happen when run using VSC. These Exceptions don't occur if run out side of VSC.

@fabioz
Copy link
Collaborator

fabioz commented Oct 12, 2021

@sknowles0143 they do happen when running outside of VSC, the only difference is that when running inside of VSC they are shown to you due to the User Uncaught Exceptions.

I believe you're confusing this with Uncaught Exceptions.

i.e.: User Uncaught Exceptions appear when an exception is raised in user code and ends up being handled inside some library, whereas Uncaught Exceptions appear when the exception isn't handled anywhere (which would show only when a thread/program would finish due to it).

@fabioz
Copy link
Collaborator

fabioz commented Oct 12, 2021

i.e.: User Uncaught Exceptions actually means: Exceptions raised from user code handled in library code

Maybe Library Caught Exceptions could be a better name?

@RobertCsordas
Copy link
Author

Thanks a lot! Disabling User Uncaught Exceptions solved the issue. Maybe add some warning saying that the exception is because of this new feature. I lost like 5 hours figuring out that it is not actually my code that is not working properly and almost went crazy figuring out what's happening. The worst thing is that isinstance checked internally hasattr for every member in random order, so the error message was always about a random member, and no code trying accessing those members was visible anywhere.

Can you clarify why isinstance causes the debugger to stop, but hasattr itself does not? Hasattr is based on the AttributeError exception, and it is also an interface between a library and the user code.

@fabioz
Copy link
Collaborator

fabioz commented Oct 13, 2021

Can you clarify why isinstance causes the debugger to stop, but hasattr itself does not? Hasattr is based on the AttributeError exception, and it is also an interface between a library and the user code.

hasattr will catch the AttributeError exception and return False. No library code will be executed as it's a builtin. isinstance will throw the exception when trying to access information on the class (and then the exception will end up being handled in library code afterwards).

@adebuyss
Copy link

adebuyss commented Oct 13, 2021

This is also an issue when using @Property decorators. Since the debugger appears to to now call hasattr on an uninstantiated object on file import, this can cause exceptions or other bad behavior. I am seeing an import loop and other problems due to this bad behavior. For instance, any accessor using 'self' is likely to throw an exception now since the 'self' argument in this case is really 'cls'.

@int19h
Copy link
Collaborator

int19h commented Oct 13, 2021

Given the issues that came up so far, I agree that we should disable this by default.

Going forward, we can try to come up with some heuristics to avoid hitting this for the more common Python patterns, and then flip it back if those prove to be sufficient in real world usage.

@int19h int19h self-assigned this Oct 13, 2021
@lordmauve
Copy link

What constitutes "user code" vs "library code"? We're seeing the debugger stop on exceptions that are fully raised and caught in library code.

@fabioz
Copy link
Collaborator

fabioz commented Oct 14, 2021

What constitutes "user code" vs "library code"? We're seeing the debugger stop on exceptions that are fully raised and caught in library code.

In general anything in site-packages and lib is library code, anything else is user code...

So, any exception that at some point passes through user code (even if originally raised in library code) and then keeps on being raised until it reaches library code is considered here.

In general, this is nice for test frameworks, which can stop at caught exceptions inside those -- and can under some circumstances -- show actual errors of unexpected exceptions, but it seems it's also common under some frameworks to work this way (so, the default should probably be opt in instead of opt out).

A second step as @int19h mentioned would be trying to discover those common scenarios to skip those, but this is probably a much harder task (given that finding which are the common cases where this shouldn't be reported is hard).

Given that initially this was done to support showing exceptions that are handled in unit-testing frameworks, maybe we could whitelist for those frameworks instead of blacklisting other scenarios (in which case the breakpoint would be renamed to something as "Exceptions caught by test frameworks").

@int19h int19h changed the title Should User Uncaught Exceptions setting be false by default? User Uncaught Exceptions setting should be false by default Oct 18, 2021
@brettcannon
Copy link
Member

Should this be closed? It was listed as fixed in the 1.5.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants