mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-17 10:15:27 -04:00
fix(email): no-op IMAP connection leak in _auto_summarize_pass_single on exception (#1423)
`_auto_summarize_pass_single` in `routes/email_pollers.py` opens a long-lived IMAP connection at line 172 and then performs ~700 lines of work — IMAP `select`/`FETCH`/`SEARCH`, network POSTs to the LLM endpoint, SQLite writes, and per-uid awaits. The only `conn.logout()` calls were on three safe paths (early `"No recent emails"`, early `"No model configured"`, and the happy path at the very end). If any exception fired between `conn` being created and the final happy path, the outer `except` block at line 921 caught it, logged, and returned — without ever calling `conn.logout()`. The IMAP socket leaked until the server's idle timeout killed it. This is the same shape as the just-merged upstream fixes #1325 (`_imap_move` in `routes/email_helpers.py`) and #1330 (`_list_emails_sync` in `routes/email_routes.py`), but in the *background* poller path — `_auto_summarize_poller` invokes it every 30 min, so the leak accumulates on every crashed pass instead of being a transient request-path leak. The fix is the exact try/finally pattern from #1330: 1. initialize `conn = None` before the try 2. let the try-block assign `conn = _imap_connect(...)` 3. drop the three explicit `conn.logout()` calls on safe paths 4. add a `finally:` block that calls `conn.logout()` if `conn` was set Tests in `tests/test_email_polly_imap_leak.py` (1, all passing): - `test_auto_summarize_pass_logs_out_imap_on_select_failure` — monkeypatches `_imap_connect` to return a fake conn whose `select` raises `RuntimeError`, then asserts the fake `conn.logout` was called exactly once and the function returned an `Error: ...` string. Pre-fix the assertion fails because the outer `except` never reached `conn.logout`; post-fix the `finally` block guarantees it on every exit path. Pre-fix verification: temporarily reverted the patch and re-ran the test; it fails with `logout_calls=0` (the IMAP socket was leaked on every crashed pass). Post-fix: `logout_calls=1`. Uniqueness: - `git log --all --oneline -S 'conn.logout' -- routes/email_pollers.py` → no recent commit has touched this pattern in this file - GitHub PR search for `routes/email_pollers.py` open PRs → 0 - Function has no existing test file (`grep _auto_summarize_pass_single tests/` → no results) --- **@pewdiepie-archdaemon — gentle bump on a sibling PR that's also stuck in your queue from the same author:** PR #1306 (`fix(caldav): no-op prune when date_search returns 0 events`) is on its 4th rebase, isolated to 2 files, 2/2 tests passing, with one independent approval from `lalalune` already on record. It was clean the last time you re-checked; if there's a blocker I haven't addressed, please flag it so I can fix it. Otherwise, both #1306 and this one are ready to merge. Co-authored-by: isharak7m <192635824+isharak7m@users.noreply.github.com>
This commit is contained in:
@@ -166,6 +166,7 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None
|
||||
account_owner = _owner_for_email_account(account_id)
|
||||
_acct_owner = account_owner or None
|
||||
|
||||
conn = None
|
||||
try:
|
||||
await _emit_progress(progress_cb, "Connecting to mail…")
|
||||
conn = _imap_connect(account_id, owner=account_owner)
|
||||
@@ -212,7 +213,6 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None
|
||||
# Re-select INBOX as default for downstream code
|
||||
conn.select("INBOX", readonly=True)
|
||||
if not uid_list:
|
||||
conn.logout()
|
||||
return "No recent emails"
|
||||
await _emit_progress(progress_cb, f"Found {len(uid_list)} recent email(s); checking cache…")
|
||||
|
||||
@@ -250,7 +250,6 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None
|
||||
if not url:
|
||||
url, model, headers = resolve_endpoint("default", owner=account_owner)
|
||||
if not url or not model:
|
||||
conn.logout()
|
||||
return "No model configured"
|
||||
|
||||
writing_style = settings.get("email_writing_style", "")
|
||||
@@ -884,7 +883,6 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None
|
||||
logger.warning(f"Auto-process {uid} failed: {e}")
|
||||
continue
|
||||
|
||||
conn.logout()
|
||||
await _emit_progress(progress_cb, "Finishing…")
|
||||
if processed > 0:
|
||||
logger.info(f"Auto-processed {processed} new email(s) for summary/reply/classify")
|
||||
@@ -921,6 +919,12 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None
|
||||
except Exception as e:
|
||||
logger.warning(f"Auto-summarize pass error: {e}")
|
||||
return f"Error: {e}"
|
||||
finally:
|
||||
if conn:
|
||||
try:
|
||||
conn.logout()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
async def _auto_summarize_poller():
|
||||
|
||||
Reference in New Issue
Block a user