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

Output error description on analysis errors #84

Closed
wants to merge 1 commit into from
Closed

Output error description on analysis errors #84

wants to merge 1 commit into from

Conversation

grigoryvp
Copy link
Contributor

'error' object is something like 'RangeError', it doesn't have any 'properties' in JSON sense and will yield a "{}" string. But it do have toString() method, so casting it to string will produce a meaningful error description.

'error' object is something like 'RangeError', it doesn't have any 'properties' in JSON sense and will yield a "{}" string. But it do have toString() method, so casting it to string will produce a meaningful error description.
@erictraut
Copy link
Collaborator

This code path is taken when an unexpected exception is thrown. We can't make any assumptions about the exception type because it's unexpected. It might be a stack overflow, a dereference of an undefined variable, etc. For that reason, I don't think we can make the assumption you're making here.

@erictraut
Copy link
Collaborator

Out of curiosity, have you seen this code path taken? If so, I'd love to know about it so I can diagnose the underlying problem.

@grigoryvp
Copy link
Contributor Author

This code path is taken when an unexpected exception is thrown. We can't make any assumptions about the exception type because it's unexpected. It might be a stack overflow, a dereference of an undefined variable, etc. For that reason, I don't think we can make the assumption you're making here.

Reasonable. Any way to detect that this is some internal exception like RangeError? Maybe parent class check? Seeing "{}" as an error result is sad for end-user.

@grigoryvp
Copy link
Contributor Author

Out of curiosity, have you seen this code path taken? If so, I'd love to know about it so I can diagnose the underlying problem.

Not an actual problem. I'm testing LSP support in VSCode and generating large codebases for pyright to chew on. I saw this exception then my codebase includes more "calls" between files than Python allows (>140). BTW, "mypy" is ok with that.

@erictraut
Copy link
Collaborator

The error information should already include the stack crawl for the exception, which is typically more useful in diagnosing the error than any other information.

You said "includes more calls between files than Python allows (>140)". Can you elaborate? What do you mean by "calls between files"? I wasn't aware of any intrinsic limits that Python places on the number of calls.

@grigoryvp
Copy link
Contributor Author

The error information should already include the stack crawl for the exception, which is typically more useful in diagnosing the error than any other information.

Nope. It looks like this. No call stack, just "{}" in your version (or "RangeError: Maximum call stack size exceeded") with my adjustment:

Screenshot 2019-04-24 at 19 33 59

You said "includes more calls between files than Python allows (>140)". Can you elaborate? What do you mean by "calls between files"? I wasn't aware of any intrinsic limits that Python places on the number of calls.

Python function/method calls are stack-based and the language will throw "stack overflow" exception after ~140 consecutive calls (depends on version, architecture, memory etc). So the code i'm testing is not a valid Python code. Just a lot of files to put some pressure on a tool.

@erictraut
Copy link
Collaborator

In this case, the crash is due to infinite recursion in the pyright code, not in the Python code it's analyzing. So this is clearly a bug in pyright, and I'd like to get it fixed. Would you be willing to share your sample code that's causing it to occur? If not, perhaps you could explain at a high level what you're doing that exposes this bug? Thanks!

@grigoryvp
Copy link
Contributor Author

In this case, the crash is due to infinite recursion in the pyright code, not in the Python code it's analyzing. So this is clearly a bug in pyright, and I'd like to get it fixed. Would you be willing to share your sample code that's causing it to occur? If not, perhaps you could explain at a high level what you're doing that exposes this bug? Thanks!

Sounds reasonable. I created some code and reproduction steps for you: #85

What about "Error performing analysis: {}" output? Any ideas how this "{}" can be changed for something more user-friendly? Maybe, I can check exception object has "toString()" method and call it for text representation if available, instead of JSON.stringify()?

@erictraut
Copy link
Collaborator

I'm not so concerned about making this error user-friendly because it should never occur. :) It's a bug in the analyzer.

@erictraut erictraut closed this Apr 25, 2019
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

Successfully merging this pull request may close these issues.

2 participants