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] Save caught exception as base_exc attribute. #265

Closed
wants to merge 1 commit into from

Conversation

rbdixon
Copy link
Contributor

@rbdixon rbdixon commented Aug 31, 2018

Reports may wish to make a decision based on the nature of the
underlying exception. In addition to the traceback the CatchedException
also retains the underlying exception as the base_exc attribute.

This is useful for custom tooling that is based on doit. If the task has already displayed a comprehensive error message to the user the Python stack trace is confusing. My custom reporter looks at the base exception and will suppress the stack trace as needed.

Reports may wish to make a decision based on the nature of the
underlying exception. In addition to the traceback the CatchedException
also retains the underlying exception as the base_exc attribute.
@coveralls
Copy link

Coverage Status

Coverage increased (+3.0e-05%) to 99.743% when pulling ca7dce0 on rbdixon:save_exception into 85c995c on pydoit:master.

@rbdixon
Copy link
Contributor Author

rbdixon commented Aug 31, 2018

The build failures look to be something unrelated to this one-line change.

@schettino72
Copy link
Member

In addition to the traceback the CatchedException also retains the underlying exception as the base_exc attribute.

I think this might be a problem. As far as I remember CatchedException is pickled and passed when using a parallel multi-process runner. So if the original Exception contains and object that can not be pickled this would break the runner...

Could you please try running some sample with multi-process?
I did not double-check this is really a problem and I could be wrong.
Note that if a base exception is pickleable you would not be able to guarantee that every Exception is pickleable.

This is useful for custom tooling that is based on doit. If the task has already displayed a comprehensive error message to the user the Python stack trace is confusing. My custom reporter looks at the base exception and will suppress the stack trace as needed.

A task might also return a plain False value to indicate that it has failed.
So (maybe) another option for your use-case would be just for you to wrap this logic in the task's action itself without relying on changes in the reporter.

So I am not sure about accepting this PR...
I would be ok on adding more attributes to CachedException as long as they are pickleable.

@schettino72 schettino72 self-requested a review September 1, 2018 21:56
@rbdixon
Copy link
Contributor Author

rbdixon commented Sep 4, 2018

ok, thanks. I have not tracked the multi-process work so I was not aware of this.

@rbdixon rbdixon closed this Sep 4, 2018
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.

3 participants