cole / aiosmtplib

asyncio smtplib implementation
MIT License
346 stars 49 forks source link

client.connect() call hangs after server timeout #243

Open ThirVondukr opened 6 months ago

ThirVondukr commented 6 months ago

Describe the bug It seems like at some point my smtp client is losing connection client.is_connected is None, and it hangs indefinitely if I call client.connect(). That problem may be related to specific SMTP server I'm using, but I don't know 🤷.

My question is: I keep SMTP instance in memory indefinitely, is that okay, or should I create a new client each time I need to send a message?

cole commented 5 months ago

client.is_connected should always be a boolean, and client.connect() shouldn't hang. Any idea what it's hanging on?

You can avoid the whole problem by using the send coroutine and creating a new connection each time, which is probably better practice anyway. But if you do have any more information, please provide it 🙂

ThirVondukr commented 5 months ago

I'm not sure why client was hanging, that may have something to do with the SMTP server we use, which I have no info about. For now I fixed it by creating a new client each time, which seems to work 🤔

ThirVondukr commented 5 months ago

Could you explain why creating a new connection would be considered best practice by the way?

cole commented 5 months ago

It's just a simple way to avoid a lot of error states from server timeouts, etc.

I wonder if there is an internal lock in the library that's getting stuck, I'll see if I can add some test cases around that.

ThirVondukr commented 5 months ago

I tried writing a context manager like this:

@contextlib.asynccontextmanager
async def reconnect_smtp_client(
    client: aiosmtplib.SMTP,
    timeout: float = 15,
) -> AsyncIterator[None]:
    try:
        if not client.is_connected:
            logging.warning("SMTP client is not connected, reconnecting.")
            async with asyncio.timeout(timeout):
                await client.connect()
        yield
    except SMTPServerDisconnected:
        logging.warning("Caught SMTPServerDisconnected exception, reconnecting.")
        async with asyncio.timeout(timeout):
            await client.connect()
        raise

But as I said it hanged on client.connect() indefinitely (initially asyncio.timeout wasn't there)

alex-pirogov commented 3 months ago

im also facing this issue my idea is to keep SMTP client instance in memory because i dont want to spend time connecting & doing login to a server each time i want to send a message

my steps to reproduce are:

  1. create SMTP client instance
    smtp_client = SMTP(...)
  2. connect to an SMTP server (i use mailslurper for development) and send a message
    
    if not smtp_client.is_connected:
    await smtp_client.connect(timeout=10)

smtp_client.send_message(message)


3. intentionally stop SMTP server
4. try to repeat step 2. when calling `await smtp_client.connect` the program hangs infinitely
ThirVondukr commented 3 months ago

I think that's the exact problem I had

alex-pirogov commented 3 months ago

i did some digging and found out that this is the place where program hangs so we need to explicitly release _connect_lock in order to be able to reconnect again this code seems to work

if not smtp_client.is_connected:
    if smtp_client._connect_lock and smtp_client._connect_lock.locked():
        smtp_client.close()

    await smtp_client.connect()