On 29/04/2021 07:36, Glyph wrote:
In my tests at least, TLSMemoryBIOProtocol.connectionLost is doing exactly the right thing - it is called with an unhelpful reason, but substitutes back in the helpful reason which has already been stashed.
Rather, the problem, as I see it, is that it's not TLSMemoryBIOProtocol.connectionLost that calls Factory.clientConnectionLost. That is done by tcp.Client.connectionLost, via one of tcp.Client's myriad of base classes, at https://github.com/twisted/twisted/blob/3c868ac11786eef7ea269caa3056f0085412.... Of course, that doesn't get the benefit of TLSMemoryBIOProtocol's reason switcheroo.
I'm still not quite sure who is in the wrong here.
Aah, yeah, this is a weird quirk of the ancient-style layering in the SMTP code :-|. The way this /should/ work is by using HostnameEndpoint.
I'm not sure /exactly/ where we're going off the rails, but by using /both /the old 'startTLS' style of starting a TLS connection, as well as relying on ClientFactory rather than an Endpoint of some kind, means that we're getting this duplicate notification; the one that you get to Protocol.connectionLost will come from TLS and have useful information, but the one that goes to the Connector will be coming straight from TCP.
The right thing to fix here, I think, is to ignore clientConnectionLost entirely, and instead to have the protocol object relay its failure to some other differently-named method on SMTPSenderFactory.
Right! That sounds plausible, and certainly gives me some places to poke. I'll have another look later. Thanks very much! R