Skip to content

Fixed "AttributeError: 'NoneType' object has no attribute 'close'" for SmtpHook #62409

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.

Sign up for GitHub

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

Draft
TopCoder2K wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Fixed "AttributeError: 'NoneType' object has no attribute 'close'" for SmtpHook #62409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a5fc4d
Fixed the bug, added a test
TopCoder2K Feb 24, 2026
ac9a550
Fixed the other 3 retry loops and updated tests
TopCoder2K Feb 26, 2026
e1b8354
Made the documentation consistent
TopCoder2K Feb 27, 2026
337da23
Made retry_limit consistent across all use of SMTP
TopCoder2K Feb 27, 2026
e426ce8
Fixed line length
TopCoder2K Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion airflow-core/src/airflow/config_templates/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2294,7 +2294,8 @@ smtp:
default: "30"
smtp_retry_limit:
description: |
Defines the maximum number of times Airflow will attempt to connect to the SMTP server.
Defines the number of times Airflow will attempt to connect to the SMTP server after the first
attempt.
version_added: 2.0.0
type: integer
example: ~
Expand Down
2 changes: 1 addition & 1 deletion airflow-core/src/airflow/utils/email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def send_mime_email(
log.debug("No user/password found for SMTP, so logging in with no authentication.")

if not dryrun:
for attempt in range(1, smtp_retry_limit + 1):
for attempt in range(0, smtp_retry_limit + 1):
log.info("Email alerting: attempt %s", str(attempt))
try:
smtp_conn = _get_smtp_connection(smtp_host, smtp_port, smtp_timeout, smtp_ssl)
Expand Down
6 changes: 3 additions & 3 deletions airflow-core/tests/unit/utils/test_email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ def test_send_mime_complete_failure(self, mock_smtp: mock.Mock, mock_smtp_ssl: m
port=conf.getint("smtp", "SMTP_PORT"),
timeout=conf.getint("smtp", "SMTP_TIMEOUT"),
)
assert mock_smtp.call_count == conf.getint("smtp", "SMTP_RETRY_LIMIT")
assert mock_smtp.call_count == conf.getint("smtp", "SMTP_RETRY_LIMIT") + 1
assert not mock_smtp_ssl.called
assert not mock_smtp.return_value.starttls.called
assert not mock_smtp.return_value.login.called
Expand All @@ -348,7 +348,7 @@ def test_send_mime_ssl_complete_failure(self, create_default_context, mock_smtp,
context=create_default_context.return_value,
)
assert create_default_context.called
assert mock_smtp_ssl.call_count == conf.getint("smtp", "SMTP_RETRY_LIMIT")
assert mock_smtp_ssl.call_count == conf.getint("smtp", "SMTP_RETRY_LIMIT") + 1
assert not mock_smtp.called
assert not mock_smtp_ssl.return_value.starttls.called
assert not mock_smtp_ssl.return_value.login.called
Expand Down Expand Up @@ -377,7 +377,7 @@ def test_send_mime_custom_timeout_retrylimit(self, mock_smtp, mock_smtp_ssl):
host=conf.get("smtp", "SMTP_HOST"), port=conf.getint("smtp", "SMTP_PORT"), timeout=custom_timeout
)
assert not mock_smtp_ssl.called
assert mock_smtp.call_count == 10
assert mock_smtp.call_count == custom_retry_limit + 1

@mock.patch("smtplib.SMTP_SSL")
@mock.patch("smtplib.SMTP")
Expand Down
2 changes: 1 addition & 1 deletion providers/smtp/docs/connections/smtp.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Configuring the Connection
* ``disable_ssl`` *(bool)* – Disable SSL/TLS entirely. Default ``false``.
* ``disable_tls`` *(bool)* – Skip ``STARTTLS``. Default ``false``.
* ``timeout`` *(int)* – Socket timeout (seconds). Default ``30``.
* ``retry_limit`` *(int)* – Connection attempts before raising. Default ``5``.
* ``retry_limit`` *(int)* – Number of retries after the first attempt before raising. Default ``5``.
* ``ssl_context`` – ``"default"`` | ``"none"``
See :ref:`howto/connection:smtp:ssl-context`.

Expand Down
8 changes: 4 additions & 4 deletions providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def get_conn(self) -> SmtpHook:
except AirflowNotFoundException:
raise AirflowException("SMTP connection is not found.")

for attempt in range(1, self.smtp_retry_limit + 1):
for attempt in range(0, self.smtp_retry_limit + 1):
try:
self._smtp_client = self._build_client()
except smtplib.SMTPServerDisconnected:
Expand Down Expand Up @@ -163,7 +163,7 @@ async def aget_conn(self) -> SmtpHook:
except AirflowNotFoundException:
raise AirflowException("SMTP connection is not found.")

for attempt in range(1, self.smtp_retry_limit + 1):
for attempt in range(0, self.smtp_retry_limit + 1):
try:
async_client = await self._abuild_client()
self._smtp_client = async_client
Expand Down Expand Up @@ -418,7 +418,7 @@ def send_email_smtp(
# Casting here to make MyPy happy.
smtp_client = cast("smtplib.SMTP_SSL | smtplib.SMTP", self._smtp_client)

for attempt in range(1, self.smtp_retry_limit + 1):
for attempt in range(0, self.smtp_retry_limit + 1):
try:
smtp_client.sendmail(
from_addr=msg["from_email"],
Expand Down Expand Up @@ -487,7 +487,7 @@ async def asend_email_smtp(
smtp_client = cast("aiosmtplib.SMTP", self._smtp_client)

if not dryrun:
for attempt in range(1, self.smtp_retry_limit + 1):
for attempt in range(0, self.smtp_retry_limit + 1):
try:
# The async version of sendmail only supports positional arguments for some reason.
await smtp_client.sendmail(
Expand Down
43 changes: 36 additions & 7 deletions providers/smtp/tests/unit/smtp/hooks/test_smtp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@

CONN_ID_DEFAULT = "smtp_default"
CONN_ID_NONSSL = "smtp_nonssl"
CONN_ID_0_RETRIES = "smtp_0_retries"
CONN_ID_SSL_EXTRA = "smtp_ssl_extra"
CONN_ID_OAUTH = "smtp_oauth2"

Expand Down Expand Up @@ -105,6 +106,17 @@ def setup_connections(self, create_connection_without_db):
extra=json.dumps(dict(disable_ssl=True, from_email=FROM_EMAIL)),
)
)
create_connection_without_db(
Connection(
conn_id=CONN_ID_0_RETRIES,
conn_type=CONN_TYPE,
host=SMTP_HOST,
login=SMTP_LOGIN,
password=SMTP_PASSWORD,
port=NONSSL_PORT,
extra=json.dumps(dict(from_email=FROM_EMAIL, retry_limit=0, disable_ssl=True)),
)
)
create_connection_without_db(
Connection(
conn_id=CONN_ID_OAUTH,
Expand Down Expand Up @@ -133,6 +145,7 @@ def setup_connections(self, create_connection_without_db):
[
pytest.param(CONN_ID_DEFAULT, True, DEFAULT_PORT, True, id="ssl-connection"),
pytest.param(CONN_ID_NONSSL, False, NONSSL_PORT, False, id="non-ssl-connection"),
pytest.param(CONN_ID_0_RETRIES, False, NONSSL_PORT, False, id="0-retries-connection"),
],
)
@patch(smtplib_string)
Expand All @@ -143,8 +156,10 @@ def test_connect_and_disconnect(
"""Test sync connection with different configurations."""
mock_conn = _create_fake_smtp(mock_smtplib, use_ssl=use_ssl)

with SmtpHook(smtp_conn_id=conn_id):
pass
smtp_hook = SmtpHook(smtp_conn_id=conn_id)
assert smtp_hook._smtp_client is None
with smtp_hook:
assert smtp_hook._smtp_client is not None

if create_context:
assert create_default_context.called
Expand Down Expand Up @@ -387,7 +402,7 @@ def test_send_mime_ssl_complete_failure(self, mock_smtp_ssl):
html_content=TEST_BODY,
)

assert mock_smtp_ssl().sendmail.call_count == DEFAULT_RETRY_LIMIT
assert mock_smtp_ssl().sendmail.call_count == DEFAULT_RETRY_LIMIT + 1

@patch("email.message.Message.as_string")
@patch("smtplib.SMTP_SSL")
Expand Down Expand Up @@ -440,7 +455,7 @@ def test_send_mime_custom_timeout_retrylimit(
)
assert expected_call in mock_smtp_ssl.call_args_list
assert create_default_context.called
assert mock_smtp_ssl().sendmail.call_count == 10
assert mock_smtp_ssl().sendmail.call_count == custom_retry_limit + 1

@patch(smtplib_string)
def test_oauth2_auth_called(self, mock_smtplib):
Expand Down Expand Up @@ -569,6 +584,17 @@ def setup_connections(self, create_connection_without_db):
extra=json.dumps(dict(disable_ssl=True, from_email=FROM_EMAIL)),
)
)
create_connection_without_db(
Connection(
conn_id=CONN_ID_0_RETRIES,
conn_type=CONN_TYPE,
host=SMTP_HOST,
login=SMTP_LOGIN,
password=SMTP_PASSWORD,
port=NONSSL_PORT,
extra=json.dumps(dict(from_email=FROM_EMAIL, retry_limit=0, disable_ssl=True)),
)
)

@pytest.fixture
def mock_smtp_client(self):
Expand Down Expand Up @@ -617,14 +643,17 @@ def _create_fake_async_smtp(mock_smtp):
[
pytest.param(CONN_ID_NONSSL, NONSSL_PORT, False, id="non-ssl-connection"),
pytest.param(CONN_ID_DEFAULT, DEFAULT_PORT, True, id="ssl-connection"),
pytest.param(CONN_ID_0_RETRIES, NONSSL_PORT, False, id="0-retries-connection"),
],
)
async def test_async_connection(
self, mock_smtp, mock_smtp_client, mock_get_connection, conn_id, expected_port, expected_ssl
):
"""Test async connection with different configurations."""
async with SmtpHook(smtp_conn_id=conn_id) as hook:
assert hook is not None
smtp_hook = SmtpHook(smtp_conn_id=conn_id)
assert smtp_hook._smtp_client is None
async with smtp_hook:
assert smtp_hook._smtp_client is not None

mock_smtp.assert_called_once_with(
hostname=SMTP_HOST,
Expand Down Expand Up @@ -667,7 +696,7 @@ async def test_async_send_email(self, mock_smtp, mock_smtp_client, mock_get_conn
False,
id="success_after_retries",
),
pytest.param(SERVER_DISCONNECTED_ERROR, DEFAULT_RETRY_LIMIT, True, id="max_retries_exceeded"),
pytest.param(SERVER_DISCONNECTED_ERROR, DEFAULT_RETRY_LIMIT + 1, True, id="max_retries_exceeded"),
],
)
@pytest.mark.asyncio
Expand Down