Skip to content

Add traceback when setting exception on an excepted Future#113

Merged
sphuber merged 1 commit intodevelopfrom
fix_111_full_traceback_excepted_future
Jun 6, 2019
Merged

Add traceback when setting exception on an excepted Future#113
sphuber merged 1 commit intodevelopfrom
fix_111_full_traceback_excepted_future

Conversation

@sphuber
Copy link
Copy Markdown
Collaborator

@sphuber sphuber commented Jun 6, 2019

Fixes #111

This was not being done, and so when the final value of the future was
resolved, only the tornado stack trace was shown and the value of the
original value. However, without the original traceback, debugging is
practically impossible.

@sphuber sphuber requested a review from ltalirz June 6, 2019 08:08
This was not being done, and so when the final value of the future was
resolved, only the tornado stack trace was shown and the value of the
original value. However, without the original traceback, debugging is
practically impossible.
@sphuber sphuber force-pushed the fix_111_full_traceback_excepted_future branch from d42a7ed to 65a148e Compare June 6, 2019 08:08
Comment thread plumpy/processes.py
self.future().set_exception(exc_info[1])
exception = exc_info[1]
exception.__traceback__ = exc_info[2]
self.future().set_exception(exception)
Copy link
Copy Markdown
Member

@ltalirz ltalirz Jun 6, 2019

Choose a reason for hiding this comment

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

Just a question: is this really the recommended way of doing this (i.e. peeling out things from exc_info by hand?)
Is there no wrapper that transforms exc_info into something you can pass to future().set_exception?

Copy link
Copy Markdown
Collaborator Author

@sphuber sphuber Jun 6, 2019

Choose a reason for hiding this comment

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

I am only doing this because the future here is a tornado concurrent.Future and the implementation of set_exception is as follows:


    def set_exception(self, exception):
        """Sets the exception of a ``Future.``"""
        self.set_exc_info(
            (exception.__class__,
             exception,
             getattr(exception, '__traceback__', None)))

I am not sure if them getting __traceback__ is standard practice, but because we were not setting it, we were not seeing the actual stack trace of excepted processes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see that the exc_info is something homegrown:

def get_exc_info(self):

Should we open an issue to consider using a standard exception class rather than separate values for exception type, message and traceback?

Anyhow, this explains why you need to do manual work here...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No that is actually standard python. If you call sys.exc_info it will return a tuple of exception type, exception and traceback. This is just a utility function to returns the stored version, because the system can override the current one if another exception occurs.

@sphuber sphuber merged commit e72c819 into develop Jun 6, 2019
@sphuber sphuber deleted the fix_111_full_traceback_excepted_future branch June 6, 2019 11:56
unkcpz pushed a commit to unkcpz/plumpy that referenced this pull request Dec 14, 2024
…am#113)

This was not being done, and so when the final value of the future was
resolved, only the tornado stack trace was shown and the value of the
original value. However, without the original traceback, debugging is
practically impossible.
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.

Traceback is not set on future result value

2 participants