Opened 3 years ago

Closed 3 years ago

#5089 regression closed fixed (fixed)

Failure debug mode breaks Deferred/Failure integration

Reported by: exarkun Owned by: itamar
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/failure-debug-5089
(diff, github, buildbot, log)
Author: itamarst Launchpad Bug:

Description

>>> from twisted.python.failure import startDebugMode
/System/Library/Frameworks/Python.framework/Versions/2.6/Extras/lib/python/zope/__init__.py:1: UserWarning: Module twisted was already imported from twisted/__init__.pyc, but /System/Library/Frameworks/Python.framework/Versions/2.6/Extras/lib/python is being added to sys.path
  __import__('pkg_resources').declare_namespace(__name__)
>>> startDebugMode()
>>> from twisted.internet.defer import maybeDeferred
>>> maybeDeferred(lambda: 1/0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "twisted/internet/defer.py", line 135, in maybeDeferred
    return fail(failure.Failure(captureVars=Deferred.debug))
TypeError: _debuginit() got an unexpected keyword argument 'captureVars'
>>> 

Change History (9)

comment:1 Changed 3 years ago by exarkun

  • Summary changed from Failure debug mode is breaks Deferred/Failure integration to Failure debug mode breaks Deferred/Failure integration

comment:2 Changed 3 years ago by exarkun

This was introduced by #5011.

comment:3 Changed 3 years ago by itamar

  • Owner changed from spiv to itamar
  • Status changed from new to assigned

Easy fix, just need to do some paperwork...

comment:4 Changed 3 years ago by itamarst

  • Author set to itamarst
  • Branch set to branches/failure-debug-5089

(In [31788]) Branching to 'failure-debug-5089'

comment:5 Changed 3 years ago by itamar

  • Keywords review added
  • Owner itamar deleted
  • Status changed from assigned to new

OK, ready for review.

comment:6 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to itamar

Hooray, thanks.

  1. It's a trivial difference, but Failure.__dict__['__init__'] will be a function before these tests run and an unbound method afterwards.
  2. It might be nice to assert that variables are or are not captured in these cases, as determined by the value passed for captureVars.
  3. There's some trailing whitespace in test_captureVars
  4. Maybe M-x set-fill-column 78 and reflow the docstrings, if you feel like it :). Also, don't say "should".
  5. Add a .misc fragment

comment:7 Changed 3 years ago by itamar

  • Keywords review added
  • Owner itamar deleted

OK, fixed all of those.

comment:8 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to itamar

.misc files don't need any content (and their content won't go into the news file). I suggested .misc instead of .bugfix because this is an unreleased regression; I don't think people reading the release notes will care about fixes for such things. Otherwise great, build results are good, please merge.

comment:9 Changed 3 years ago by itamarst

  • Resolution set to fixed
  • Status changed from new to closed

(In [31807]) Merge failure-debug-5089.

Author: itamar
Reviewer: exarkun
Fixes: #5089

Fix regression that broke t.p.failure.startDebugMode() and therefore 'trial -b'.
Also, added tests to prevent this from breaking in future.

Note: See TracTickets for help on using tickets.