Failure should adopt the current traceback if _findFailure's is empty

	@markrwilliams reported
Trac ID	trac#9004
Type	enhancement
Created	2017-01-19 01:39:36Z

twisted.python.failure.Failure attempts to find an existing Failure that wraps the current exception, replacing its own __dict__ with that Failure's __dict__. This is all well and good unless the existing Failure lacks a traceback. The new Failure necessarily lacks a traceback though a meaningful one might be extracted from the interpreter.

Consider the following program:

from twisted.logger import globalLogBeginner, textFileLogObserver, Logger
from twisted.internet.defer import inlineCallbacks
from twisted.internet.task import react
from twisted.python import failure
from twisted.internet import protocol, endpoints
import sys

log = Logger()


@inlineCallbacks
def failsToConnect(reactor):
    endpoint = endpoints.TCP4ClientEndpoint(reactor, '127.0.0.1', 0)
    yield endpoint.connect(protocol.Factory.forProtocol(protocol.Protocol))


@inlineCallbacks
def someWork(reactor):
    try:
        yield failsToConnect(reactor)
    except:
        log.failure("No traceback", failure=failure.Failure())
        exc_type, exc_value, exc_tb = sys.exc_info()
        log.failure("Has traceback",
                    failure=failure.Failure(exc_value,
                                            exc_type,
                                            exc_tb))


globalLogBeginner.beginLoggingTo([textFileLogObserver(sys.stdout)])

react(someWork)

(This will probably fail to connect to anything.)

The first Failure adopts the Failure, which was constructed, not thrown, in tcp.Port. Consequently the log message lacks any traceback:

2017-01-21T10:40:46-0500 [__main__#critical] No traceback
        Traceback (most recent call last):
        Failure: twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 111: Connection refused.

The second Failure receives the current traceback. This doesn't point at the originating code, but does point at the Failure's construction site. That can be very useful in debugging!

2017-01-21T10:40:46-0500 [__main__#critical] Has traceback
        Traceback (most recent call last):
          File "/home/exarkun/Work/LeastAuthority/twisted/src/twisted/internet/defer.py", line 1355, in gotResult
            _inlineCallbacks(r, g, deferred)
          File "/home/exarkun/Work/LeastAuthority/twisted/src/twisted/internet/defer.py", line 1297, in _inlineCallbacks
            result = result.throwExceptionIntoGenerator(g)
          File "/home/exarkun/Work/LeastAuthority/twisted/src/twisted/python/failure.py", line 393, in throwExceptionIntoGenerator
            return g.throw(self.type, self.value, self.tb)
          File "what.py", line 27, in someWork
            exc_tb))
        --- <exception caught here> ---
          File "what.py", line 20, in someWork
            yield failsToConnect(reactor)
        twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 111: Connection refused.

Failure._findFailure should not return a Failure that lacks a traceback. This will prevent Failure from dropping useful debugging information.

trac-id__9004 9004
type__enhancement enhancement
reporter__markrwilliams markrwilliams
priority__normal normal
milestone__None None
branch__ 
branch_author__ 
status__new new
resolution__None None
component__core core
keywords__None None
time__1484789976897055 1484789976897055
changetime__1485013763991292 1485013763991292
version__None None
owner__None None

Start in twisted/python/failure.py at Failure._findFailure and reproduce the traceback behavior with the example program in the issue. Verify that a Failure without a traceback is not adopted when the current exception has a traceback, while existing traceback information remains available.