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

HookMap.run swallows tracebacks #1770

Closed
ddaanet opened this issue Feb 20, 2019 · 2 comments

Comments

@ddaanet
Copy link

commented Feb 20, 2019

I'm submitting a ...

  • bug report
  • feature request
  • question about the decisions made in the repository

Do you want to request a feature or report a bug?

Report a bug.

What is the current behavior?

HookMap.run does not preserve traceback information when passing an exception.

If the exception is not HTTPError, HTTPRedirect, InternalRedirect, KeyboardInterrupt or SystemExit, it is logged with traceback through cherrypy.log, but then the exception is re-raised without preserving the original traceback.

If the current behavior is a bug, please provide the steps to reproduce and if possible a screenshots and logs of the problem. If you can, show us your code.

I could provide all that if that's really needed, but come on...

What is the expected behavior?

Re-raise exceptions from hooks in a way that preserves the original traceback.

What is the motivation / use case for changing the behavior?

Making it easier to debug tools from automated tests using webtest. In my environment, logging is suppressed to reduce noise, but then tracebacks originating from tools are lost.

Please tell us about your environment:

  • Cheroot version: 6.5.4
  • CherryPy version: 17.4.1
  • Python version: 2.7.15
  • OS: Not relevant
  • Browser: Not relevant

Other information

The fix is trivial,
https://github.com/cherrypy/cherrypy/blob/master/cherrypy/_cprequest.py#L114
Replace with a bare raise statement.

The bare raise statement was introduced in Python 1.5.1p1… It provides exactly the semantics wanted here.

https://docs.python.org/release/1.5.1p1/ref/raise.html
https://docs.python.org/release/2.7.15/reference/simple_stmts.html#the-raise-statement
https://docs.python.org/release/3.7.2/reference/simple_stmts.html#the-raise-statement

PR upcoming.

@ddaanet

This comment has been minimized.

Copy link
Author

commented Feb 20, 2019

By the way, it would be nice to backport this to the 17 line. I understand it is not a security issue, but seriously...

@ddaanet ddaanet referenced this issue Feb 20, 2019
5 of 15 tasks complete
@jaraco

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

FYI, for this bug or any that you would like to see in the 17 line, just target the PR at the maint/17.x branch, all commits which should be merged back into master.

@jaraco jaraco referenced this issue Sep 10, 2019
6 of 15 tasks complete
@jaraco jaraco closed this in 6f09ac7 Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.