-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix TypeError: exceptions must derive from BaseException #329
Conversation
It was probably important to someone, somewhere, at some point. But if we trace back this piece of code, a first commit included six to reraise the same error and second commit removed six and added a python error ( da5fa09 ) I am not a python expert, and it seems very strange to catch an error to reraise it right after without modification. I didn't find the benefits for doing it, maybe you can help me understand how it is important ? And yes, I should have removed the comment above too. But still, I'm wondering if this is necessary to reraise the same error or not. |
Looking back through the wsgi-intercept code it appears that the code is not rerasiing without modification. WSGIAppError wraps another exception. What the re-raise is doing is raising the wrapped exception, because (as the comment says) we don't want two layers of exceptions when debugging an application we are developing with TDD and testing via gabbi+wsgi-intercept. What was the original issue you were fixing? That is, under what circumstances are you seeing the TypeError? |
You are right. Thanks for pointing me to the right direction, it was not re-raising the same error. To reproduce the problem I had with TypeError:
Here is what happened before, when there was an error:
Here is what the new patch does:
I am not sure it looks perfect, but I think it is more readable and shows the traceback wrapped in WSGIAppError as intended. |
Ah, cool, thanks for the detailed explanation, this looks good and has the desired behaviour. |
I'll give it a closer look tomorrow just to be sure but then merge it and do a new release. Thanks again. |
Hello,
There was an issue with the "raise" line.
I removed it and cleaned a bit, but maybe you want to raise a new error instead of ignoring the WSGIAppError ?
Bye