Skip to content

Commit c36b54e

Browse files
authored
Merge pull request #485 from tutorcruncher/catch-exception
Catch spam service exception
2 parents f3b81f3 + 12298e4 commit c36b54e

File tree

4 files changed

+85
-12
lines changed

4 files changed

+85
-12
lines changed

setup.cfg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ filterwarnings =
66
timeout = 20
77
markers =
88
spam: Using this marker on a test will mark the emails that test as spam.
9+
spam_service_error: Using this marker on a test will mark the emails that test as spam service error.
910

1011
[flake8]
1112
max-line-length = 120

src/spam/email_checker.py

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import logging
22
import re
33
from html import unescape
4+
from openai import OpenAIError
45
from typing import Optional, Union
56

67
from src.render.main import MessageDef, render_email
78
from src.schemas.messages import EmailSendModel
8-
from src.spam.services import OpenAISpamEmailService, SpamCacheService
9+
from src.spam.services import OpenAISpamEmailService, SpamCacheService, SpamCheckResult
910

1011
logger = logging.getLogger('spam.email_checker')
1112

@@ -57,21 +58,38 @@ async def check_spam(self, m: EmailSendModel):
5758
company_name = m.context.get("company_name", "no_company")
5859
subject = email_info.subject
5960

60-
spam_result = await self.spam_service.is_spam_email(email_info, company_name)
61+
try:
62+
spam_result = await self.spam_service.is_spam_email(email_info, company_name)
63+
# Cache all results (both spam and non-spam) - only when successful
64+
await self.cache_service.set(m, spam_result)
6165

62-
# Cache all results (both spam and non-spam)
63-
await self.cache_service.set(m, spam_result)
64-
65-
if spam_result.spam:
66+
if spam_result.spam:
67+
logger.error(
68+
"Email flagged as spam",
69+
extra={
70+
"reason": spam_result.reason,
71+
"number of recipients": len(m.recipients),
72+
"subject": subject,
73+
"company": company_name,
74+
"company_code": m.company_code,
75+
"email_main_body": _clean_html_body(context.get('main_message__render')) or 'no main body',
76+
},
77+
)
78+
except OpenAIError as e:
79+
# Use the same logging structure for consistency
6680
logger.error(
67-
"Email flagged as spam",
81+
"LLM Provider Error during spam check",
6882
extra={
69-
"reason": spam_result.reason,
70-
"number of recipients": len(m.recipients),
71-
"subject": subject,
83+
"reason": str(e),
84+
"subject": email_info.subject,
85+
"email_main_body": _clean_html_body(context.get('main_message__render')) or 'no main body',
7286
"company": company_name,
7387
"company_code": m.company_code,
74-
"email_main_body": _clean_html_body(context.get('main_message__render')) or 'no main body',
7588
},
7689
)
90+
# Return a safe default when spam check fails
91+
spam_result = SpamCheckResult(
92+
spam=False, reason="Spam check failed - defaulting to not spam due to service error"
93+
)
94+
7795
return spam_result

tests/conftest.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,13 @@ class FakeResponse:
308308

309309
# Create a fake client with a mocked responses.parse method
310310
fake_client = AsyncMock()
311-
fake_client.responses.parse.return_value = FakeResponse()
311+
if 'spam_service_error' in request.keywords:
312+
# This will RAISE OpenAIError when fake_client.responses.parse() is called
313+
from openai import OpenAIError
314+
315+
fake_client.responses.parse.side_effect = OpenAIError('Openai test error')
316+
else:
317+
fake_client.responses.parse.return_value = FakeResponse()
312318

313319
fake_service = OpenAISpamEmailService(client=fake_client)
314320
fake_cache = SpamCacheService(glove.redis)

tests/test_email.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,3 +1213,51 @@ def test_spam_logging_includes_body(cli: TestClient, sync_db: SyncDb, worker, ca
12131213
body == 'Hi {{ recipient_first_name }}, Dont miss out on FREE MONEY! '
12141214
'Click [here]({{ login_link }}) now! Regards, {{ company_name }}'
12151215
)
1216+
1217+
1218+
@pytest.mark.spam_service_error
1219+
def test_openai_service_error(cli: TestClient, sync_db: SyncDb, worker, caplog):
1220+
caplog.set_level(logging.ERROR, logger='spam.email_checker')
1221+
1222+
recipients = []
1223+
for i in range(21):
1224+
recipients.append(
1225+
{
1226+
'first_name': f'User{i}',
1227+
'last_name': f'Last{i}',
1228+
'address': f'user{i}@example.org',
1229+
'tags': ['test'],
1230+
}
1231+
)
1232+
1233+
data = {
1234+
'uid': str(uuid4()),
1235+
'company_code': 'foobar',
1236+
'from_address': 'Spammer <[email protected]>',
1237+
'method': 'email-test',
1238+
'subject_template': 'Urgent: {{ company_name }} Alert!',
1239+
'main_template': '{{{ main_message }}}',
1240+
'context': {
1241+
'main_message__render': 'Hi {{ recipient_first_name }},\n\nDont miss out on <b>FREE MONEY</b>! '
1242+
'Click [here]({{ login_link }}) now!\n\nRegards,\n{{ company_name }}',
1243+
'company_name': 'TestCorp',
1244+
'login_link': 'https://spam.example.com/click',
1245+
},
1246+
'recipients': recipients,
1247+
}
1248+
1249+
r = cli.post('/send/email/', json=data, headers={'Authorization': 'testing-key'})
1250+
assert r.status_code == 201, r.text
1251+
assert worker.test_run() == len(recipients)
1252+
1253+
records = [r for r in caplog.records if r.name == 'spam.email_checker' and r.levelno == logging.ERROR]
1254+
assert len(records) == 1
1255+
record = records[0]
1256+
assert record.reason == 'Openai test error'
1257+
assert record.subject == 'Urgent: TestCorp Alert!'
1258+
assert (
1259+
record.email_main_body == 'Hi {{ recipient_first_name }}, Dont miss out on FREE MONEY! '
1260+
'Click [here]({{ login_link }}) now! Regards, {{ company_name }}'
1261+
)
1262+
assert record.company == 'TestCorp'
1263+
assert record.company_code == 'foobar'

0 commit comments

Comments
 (0)