[Twisted-Python] SMTPConnectError as a side effect on connection lost

[Glyph]

> On Nov 26, 2017, at 5:10 AM, Adi Roiban <adi at roiban.ro> wrote:
> 
> On 26 November 2017 at 05:36, Glyph <glyph at twistedmatrix.com> wrote:
>> 
>>> On Nov 25, 2017, at 8:52 PM, Adi Roiban <adi at roiban.ro> wrote:
>>> 
>>> Do you see anything suspicious with this code?
>> 
>> Just that it looks like it leaves a Deferred in an error state. You said the tests weren’t “written using” trial, but are you running them with trial, or with any runner that will consider a logged failure to be a failure? If so, the failure won’t be logged until the garbage collector runs, which might be in a later test.
> 
> Many thanks for looking into that.
> 
> I dag deeper and I found the error.

Oops, looks like I was totally off base :-).

> The root cause is
> twisted.internet.posixbase._DisconnectSelectableMixin._disconnectSelectable
> 
> It starts as:
> 
>    def _disconnectSelectable(self, selectable, why, isRead, faildict={
>        error.ConnectionDone: failure.Failure(error.ConnectionDone()),
>        error.ConnectionLost: failure.Failure(error.ConnectionLost())
>        }):
>        """
>        Utility function for disconnecting a selectable.
> 
>        Supports half-close notification, isRead should be boolean indicating
>        whether error resulted from doRead().
>        """
>        self.removeReader(selectable)
>        f = faildict.get(why.__class__)
> 
> Then `f` is returned but in STMP we have:
> 
> f.value = smtp.SMTPConnectError( -1, "Unable to connect to server.")
> 
> This replaces the value for `faildict` via f.
> Any future call will return that replaced error.

Yeah, this is definitely wrong.  I don't think it occurred to anyone that someone might set the `value` attribute of an existing Failure.  More on the ticket.

> I have created a ticket for this
> 
> https://twistedmatrix.com/trac/ticket/9339
> 
> I guess that we can continue the discussion on how to fix it as part
> of that ticket.
> 
> ----------
> 
> Here is my answer to Glyph's question regarding unhandled failed deferred.
> 
> The tests are not inheriting from the Trial test case.
> 
> I can run them with trial, including with --force-gc and no leftover
> deferred is detected.
> The SMTP test is [OK].
> 
> If I reverse the order, so that the STMP tests is executed last, trial
> will execute all tests without any error.

Yeah, your diagnosis is correct, mine was wrong.

-glyph
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20171126/be3753ed/attachment-0002.html>