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

exceptions raised without a type (breaks Python2 compatibility) #58

Closed
cs01 opened this issue Dec 5, 2017 · 14 comments
Closed

exceptions raised without a type (breaks Python2 compatibility) #58

cs01 opened this issue Dec 5, 2017 · 14 comments
Assignees
Labels

Comments

@cs01
Copy link

cs01 commented Dec 5, 2017

socket.py in v2.0.1 raises an Exception with no type. This yields the error TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType.

The fix is to simply raise an exception type, such as raise ValueError.

All instances of bare raise calls should be updated to raise a specific type. This is the only one I know of though.

(noticed via cs01/gdbgui#144)

@miguelgrinberg
Copy link
Owner

Have you seen the Python 3 documentation for raise? https://docs.python.org/3/reference/simple_stmts.html#the-raise-statement

From that page:

If no expressions are present, raise re-raises the last exception that was active in the current scope.

I'm not sure I understand how you've got that error from the code you pointed at. The raise is inside an except block, so there should be an active exception in that scope.

@cs01
Copy link
Author

cs01 commented Dec 5, 2017

Agreed that Python3 is fine with it.

The issue is that Python2 does not handle it well.

@miguelgrinberg
Copy link
Owner

Can't reproduce on Py2.7 either:

~ $ python2.7
Python 2.7.14 (default, Sep 25 2017, 09:53:22)
[GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.37)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> try:
...     1/0
... except Exception:
...     print "got exception"
...     raise
...
got exception
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: integer division or modulo by zero
>>>

@cs01
Copy link
Author

cs01 commented Dec 5, 2017

I just confirmed that it works as you just described in Python 2.7.12. So now I'm confused too.

Maybe older minor versions of Python 2.7 don't behave this way?

EDIT: To be clear, I never got the error myself. A user of my tool got the error, and I'm trying to resolve it on their behalf. See cs01/gdbgui#144 for his screenshot.

@cs01
Copy link
Author

cs01 commented Dec 6, 2017

Though we don't understand why the error is occurring, it seems like it wouldn't hurt to always explicitly raise an exception. Would you be open to making that change? I can submit a pull request if you are.

@miguelgrinberg
Copy link
Owner

The problem is that I don't just want to raise an exception. I want to raise the exception that I just caught. If I raise a different exception I would be losing the context of where the original exception occurred. That's the whole point of using raise w/o any arguments.

Does your user have a way to reproduce this?

@cs01
Copy link
Author

cs01 commented Dec 6, 2017

I'm not sure, I just asked him. I will let you know if/when he responds.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Dec 9, 2017

@cs01 I think I may have figured out this problem. Seems to be an issue with Python 2.7 when multiple exceptions are raised. I extended my division by zero example above as follows:

Python 3.5.2 (default, Sep 10 2016, 08:21:44)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> try:
...     1/0
... except Exception:
...     try:
...         test()
...     except Exception:
...          pass
...     raise
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

This was Python 3. But Python 2 appears to get confused by the inner exception:

Python 2.7.12 (default, Jul  1 2016, 15:12:24)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> try:
...     1/0
... except Exception:
...     try:
...         test()
...     except Exception:
...         pass
...     raise
...
Traceback (most recent call last):
  File "<stdin>", line 5, in <module>
NameError: name 'test' is not defined

This demonstrates that any exceptions raised during the handling of another exception can mess up the exception stack, even if they are handled properly. Not completely sure how we get from this to the problem you reported, but clearly there is something going on with Python 2 that isn't expected.

I have to figure out a way to re-raise these exceptions in a way that is compatible with both Python 2 and 3. Hopefully six.raise_from() will do the right thing.

@miguelgrinberg
Copy link
Owner

@cs01 I have put a fix that I hope will solve the problem you reported. Would your user be able to install this package from git master and confirm?

@cs01
Copy link
Author

cs01 commented Dec 9, 2017

Thanks! Currently they install via pip, which pulls in engineio as a dependency. Do you have a recommended installation procedure/command I can recommend they follow to test this?

I'm pretty sure your fix will work since you seem to have identified the root cause and have tested the fix. In any case it will at least improve the situation. I would be fine if you released a new version and I updated my dependencies in setup.py, but of course it's up to you.

@miguelgrinberg
Copy link
Owner

Problem is that I have not reproduced the problem, I'm sort of "flying by instruments" here. I can see how this can happen, but can't think of a test I can build to generate the results your user reported.

Your user can run the following command after installing flask-socketio with all its dependencies to override python-engineio with the github master version:

pip install https://github.com/miguelgrinberg/python-engineio/archive/master.zip

@cs01
Copy link
Author

cs01 commented Dec 10, 2017

I will ask them and follow up in this thread when they respond. Thank you!

@cs01
Copy link
Author

cs01 commented Dec 30, 2017

Unfortunately they have not responded to my requests. I have nothing new to share at this time but wanted to let you know.

@franksands
Copy link

Hi, I was looking for answers to a problem of my own running python 2.7, got here from a google search and thought this might help.
In my code, the problem was calling raise without being in an except block:

try:
  # a lot of code here
  if something:
    raise
except:
  # error handling

This raise inside the if that was casing this "TypeError: exceptions must be old-style" for me.

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

No branches or pull requests

3 participants