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

mgr: fixes python error handling #21005

Merged
merged 1 commit into from Mar 27, 2018
Merged

Conversation

rjfd
Copy link
Contributor

@rjfd rjfd commented Mar 22, 2018

The current handle_pyerror function implementation relies in the
traceback.format_exception_only python function to format the
exception object. The problem is that this python function might also
raise an exception. This commit fixes it by enclosing that python
function call in try...catch block.

Fixes: http://tracker.ceph.com/issues/23406

Signed-off-by: Ricardo Dias rdias@suse.com

try {
formatted_list = format_exception(hexc,hval, htb);
} catch (error_already_set const &) {
return "exception raised in traceback.format_exception_only";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/format_exception_only/format_exception/g

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@rjfd rjfd force-pushed the wip-mgr-fix-error-handling branch 2 times, most recently from 07332b4 to 49575ca Compare March 22, 2018 15:40
@jcsp
Copy link
Contributor

jcsp commented Mar 23, 2018

Out of curiousity, what was the exception that format_exception_only was throwing?

@tserong
Copy link
Contributor

tserong commented Mar 23, 2018

Out of curiousity, what was the exception that format_exception_only was throwing?

I was wondering the same thing. Do we care that this piece is effectively invisible?

@rjfd
Copy link
Contributor Author

rjfd commented Mar 23, 2018

@jcsp @tserong I'm investigating a bit further the root cause of the problem to answer your question.

@rjfd rjfd force-pushed the wip-mgr-fix-error-handling branch from 49575ca to 77229b6 Compare March 23, 2018 12:45
@rjfd
Copy link
Contributor Author

rjfd commented Mar 23, 2018

@jcsp @tserong

First of all, this problem only occurs when running ceph-mgr daemon with python 3.
After investigating the root cause of the problem I found that:

  • the exception thrown while executing traceback.format_exception is only thrown when handling an exception raised by the C python API, for instance handling an execption raised while executing PyArg_ParseTuple.
  • (according to Failure in new dreload tests under Python 3.2 ipython/ipython#1626, and I confirmed that it was the case) apparently some exceptions' value generated by the C python API when raising an exception, have the str type instead of being an object of a class that subclasses BaseException. This is the cause of the exception thrown by traceback.format_exception with the following message: 'str' object has no attribute '__cause__'.

I updated the PR fix to show the exception type, plus exception value, when the traceback.format_exception raises an exception.

For instance, when calling the ceph dashboard set-login-credentials ... with the implementation described in the tracker issue, we now get the following message:
TypeError: ceph_config_set() argument 2 must be str or None, not bytes

The current `handle_pyerror` function implementation relies in the
`traceback.format_exception_only` python function to format the
exception object. The problem is that this python function might also
raise an exception. This commit fixes it by enclosing that python
function call in try...catch block.

Fixes: http://tracker.ceph.com/issues/23406

Signed-off-by: Ricardo Dias <rdias@suse.com>
@rjfd rjfd force-pushed the wip-mgr-fix-error-handling branch from 77229b6 to 072699d Compare March 26, 2018 14:22
@rjfd
Copy link
Contributor Author

rjfd commented Mar 26, 2018

@tserong removed the two unused variables from the code

@rjfd
Copy link
Contributor Author

rjfd commented Mar 27, 2018

jenkins retest this please

1 similar comment
@rjfd
Copy link
Contributor Author

rjfd commented Mar 27, 2018

jenkins retest this please

@rjfd rjfd merged commit b4a7955 into ceph:master Mar 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants