Skip to content

Unhandled exception somewhere within use of gatherResults causes test to hang #61

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

Closed
rschwiebert opened this issue Feb 26, 2019 · 6 comments

Comments

@rschwiebert
Copy link

rschwiebert commented Feb 26, 2019

Please try this:

import pytest
import pytest_twisted
from twisted.internet.defer import inlineCallbacks, Deferred, gatherResults


@inlineCallbacks
def thingy():
    d1 = Deferred()
    d2 = Deferred()

    yield gatherResults([d1, d2], consumeErrors=True)


@pytest_twisted.inlineCallbacks
def test_thingy():
    result = yield thingy()

This hangs indefinitely for me when I run pytest against it. I was having the problem with python=3.7, pytest=4.3.0 and pytest-twisted=1.9, although I initially encountered it in a context where I was running python=2.7, pytest=3.1.2, pytest-twisted=1.5.

It does not appear to make a difference what consumerErrors is set to. Of course, yielding on the deferreds sequentially and not using gatherResults works fine.

The test hangs until you CTRL-C, where you see something like this:

/usr/local/lib/python2.7/dist-packages/pytest_twisted/plugin.py:75: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

d = <Deferred at 0x7f895b2ab0e0 waiting on Deferred at 0x7f895b2ab710>

def blockon(d):
    current = greenlet.getcurrent()
    assert current is not gr_twisted, "blockon cannot be called from the twisted greenlet"
    result = []

    def cb(r):
        result.append(r)
        if greenlet.getcurrent() is not current:
            current.switch(result)

    d.addCallbacks(cb, cb)
    if not result:
        _result = gr_twisted.switch()
>           assert _result is result, "illegal switch in blockon"
E           AssertionError: illegal switch in blockon

I learned that this error message probably stemmed from an unhandled exception at this post: #4

I tried the fixture suggested recently at that post, but it did not change the problem I was having.

@altendky
Copy link
Member

Your example does hang for me as well. So does this.

import pytest
import pytest_twisted
from twisted.internet.defer import inlineCallbacks, Deferred, gatherResults


@inlineCallbacks
def thingy():
    d1 = Deferred()
    d2 = Deferred()

    yield d1
    yield d2


@pytest_twisted.inlineCallbacks
def test_thingy():
    result = yield thingy()

Wouldn't you need to .callback() the deferreds?

import pytest_twisted
from twisted.internet.defer import inlineCallbacks, Deferred, gatherResults


@inlineCallbacks
def thingy():
    d1 = Deferred()
    d2 = Deferred()

    d1.callback(1)
    d2.callback(2)

    result = yield gatherResults([d1, d2], consumeErrors=True)

    return result


@pytest_twisted.inlineCallbacks
def test_thingy():
    result = yield thingy()
    assert result == [1, 2]

The fixture just makes a 'best effort' to fail tests when you leave an errback unhandled. Such as below.

import gc

import pytest
import twisted.internet.defer
import twisted.logger


class Observer:
    def __init__(self):
        self.failures = []

    def __call__(self, event_dict):
        is_error = event_dict.get('isError')
        s = 'Unhandled error in Deferred'.casefold()
        is_unhandled = s in event_dict.get('log_format', '').casefold()

        if is_error and is_unhandled:
            self.failures.append(event_dict)

    def assert_empty(self):
        assert [] == self.failures


@pytest.fixture
def assert_no_unhandled_errbacks():
    observer = Observer()
    twisted.logger.globalLogPublisher.addObserver(observer)

    yield

    gc.collect()
    twisted.logger.globalLogPublisher.removeObserver(observer)

    observer.assert_empty()


def test_thingy(assert_no_unhandled_errbacks):
    d = twisted.internet.defer.Deferred()
    d.errback(2)

I'm guessing there's another example behind this that is closer to a 'real' problem? Or, maybe I missed something here.

Cheers,
-kyle

@rschwiebert
Copy link
Author

@altendky Well, that is odd: in the production code, when I commented out the gatherResults and yielded in sequence on the deferreds, the test ran fine. But it looks like I do see the same problem on the small example like you said.

Wouldn't you need to .callback() the deferreds?

That's the point of gatherResults, right? To fire their callbacks and return when all deferreds are fired? I think passing gatherResults already-fired callbacks makes gatherResults a no-op.

@altendky
Copy link
Member

Sure, manually calling callback() on the deferreds before gathering is a contrived way of having the deferreds fire. But something needs to if you want gatherResults() (or yielding on the deferred) to ever finish (or one needs to fail). gatherResults() isn't about defining the results of the deferreds, it is about collecting the results when they are ready.

https://twistedmatrix.com/documents/current/api/twisted.internet.defer.gatherResults.html

The returned Deferred will fire when all of the provided Deferreds have fired, or when any one of them has failed.

Neither d1 nor d2 are ever fired in the original example, nor do they fail. As such, gatherResults() should also never complete and the test hangs forever.

@rschwiebert
Copy link
Author

OK, that makes sense. I need to mock out the resolution of those deferreds. Honestly I wouldn't have posted this if my serial evaluation had failed in the original context >:( Since that one succeeded I thought I was onto something at the time.

I'll have to think here a bit about what to patch...

@altendky
Copy link
Member

If you want to share more I can take a look and see if I can help. Here or on IRC in #python or #twisted (ping me, altendky, if you go on IRC).

@rschwiebert
Copy link
Author

@altendky I wound up setting the return_value on the mock'd functions to a fired deferred, and everything works now. Perhaps not the most elegant, but it makes sense now! Thanks for your help.

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

No branches or pull requests

2 participants